From b0187862f821571c8dd71f7aa3eccee374b5d809 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 5 Nov 2020 16:00:44 +0100 Subject: [PATCH] Improve prompt notifications --- firewall/prompt.go | 103 +++++++++++++++++++++++++++++---------------- profile/profile.go | 46 ++++++++++++++++---- 2 files changed, 105 insertions(+), 44 deletions(-) diff --git a/firewall/prompt.go b/firewall/prompt.go index c9eb3c07..5a723174 100644 --- a/firewall/prompt.go +++ b/firewall/prompt.go @@ -17,15 +17,17 @@ import ( const ( // notification action IDs - permitDomainAll = "permit-domain-all" - permitDomainDistinct = "permit-domain-distinct" - denyDomainAll = "deny-domain-all" - denyDomainDistinct = "deny-domain-distinct" + allowDomainAll = "allow-domain-all" + allowDomainDistinct = "allow-domain-distinct" + blockDomainAll = "block-domain-all" + blockDomainDistinct = "block-domain-distinct" - permitIP = "permit-ip" - denyIP = "deny-ip" - permitServingIP = "permit-serving-ip" - denyServingIP = "deny-serving-ip" + allowIP = "allow-ip" + blockIP = "block-ip" + allowServingIP = "allow-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 = ¬ifications.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, - Text: "Permit", + ID: allowServingIP, + Text: "Allow", }, { - ID: denyServingIP, - Text: "Deny", + ID: blockServingIP, + 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, - Text: "Permit", + ID: allowIP, + Text: "Allow", }, { - ID: denyIP, - Text: "Deny", + ID: blockIP, + 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, - Text: "Permit all", + ID: allowDomainAll, + Text: "Allow", }, { - ID: permitDomainDistinct, - Text: "Permit", - }, - { - ID: denyDomainDistinct, - Text: "Deny", + ID: blockDomainAll, + Text: "Block", }, } } @@ -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: diff --git a/profile/profile.go b/profile/profile.go index 39479357..8bc6bf5a 100644 --- a/profile/profile.go +++ b/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 { - profile.layeredProfile.Lock() - defer profile.layeredProfile.Unlock() - - profile.layeredProfile.RevisionCounter++ + if !changed || profile.layeredProfile == nil { + return } + + 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 { - endpointList = make([]string, 0, 1) + if ok { + // 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