From fa3f873c315b12d2ff083a385e63b88634e32fa9 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 30 Oct 2020 11:54:00 +0100 Subject: [PATCH] Implement review feedback --- firewall/interception/interception_windows.go | 2 +- firewall/master.go | 6 ++- netenv/online-status.go | 6 +-- process/database.go | 9 ++-- process/process.go | 20 ++++++++ profile/get.go | 3 +- profile/profile-layered.go | 46 ++++++++++++------- 7 files changed, 64 insertions(+), 28 deletions(-) diff --git a/firewall/interception/interception_windows.go b/firewall/interception/interception_windows.go index 4be0f1a0..62f61930 100644 --- a/firewall/interception/interception_windows.go +++ b/firewall/interception/interception_windows.go @@ -78,7 +78,7 @@ func notifyDisableDNSCache() { func notifyRebootRequired() { (¬ifications.Notification{ - EventID: "interception:©windows-dnscache-reboot-required", + EventID: "interception:windows-dnscache-reboot-required", Message: "Please restart your system to complete Portmaster integration.", Type: notifications.Warning, }).Save() diff --git a/firewall/master.go b/firewall/master.go index ceed75c8..d6daf61a 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -38,7 +38,9 @@ import ( const noReasonOptionKey = "" -var deciders = []func(context.Context, *network.Connection, packet.Packet) bool{ +type deciderFn func(context.Context, *network.Connection, packet.Packet) bool + +var deciders = []deciderFn{ checkPortmasterConnection, checkSelfCommunication, checkConnectionType, @@ -152,7 +154,7 @@ func checkSelfCommunication(ctx context.Context, conn *network.Connection, pkt p if err != nil { log.Tracer(ctx).Warningf("filter: failed to find load local peer process with PID %d: %s", otherPid, err) } else if otherProcess.Pid == conn.Process().Pid { - conn.Accept("connection to self", noReasonOptionKey) + conn.Accept("process internal connection", noReasonOptionKey) conn.Internal = true return true } diff --git a/netenv/online-status.go b/netenv/online-status.go index 3d31f8bd..ba2275ca 100644 --- a/netenv/online-status.go +++ b/netenv/online-status.go @@ -227,11 +227,8 @@ func setCaptivePortal(portalURL *url.URL) { // notify cleanUpPortalNotification() - // TODO: add "open" button captivePortalNotification = notifications.Notify(¬ifications.Notification{ - EventID: fmt.Sprintf( - "netenv:captive-portal:%s", captivePortal.Domain, - ), + EventID: "netenv:captive-portal", Type: notifications.Info, Title: "Captive Portal", Category: "Core", @@ -239,6 +236,7 @@ func setCaptivePortal(portalURL *url.URL) { "Portmaster detected a captive portal at %s", captivePortal.Domain, ), + EventData: captivePortal, }) } diff --git a/process/database.go b/process/database.go index ce67f863..46191371 100644 --- a/process/database.go +++ b/process/database.go @@ -122,11 +122,12 @@ func CleanProcessStorage(activePIDs map[int]struct{}) { } // Process is inactive, start deletion process + lastSeen := p.GetLastSeen() switch { - case p.LastSeen == 0: - // add last - p.LastSeen = time.Now().Unix() - case p.LastSeen > threshold: + case lastSeen == 0: + // add last seen timestamp + p.SetLastSeen(time.Now().Unix()) + case lastSeen > threshold: // within keep period default: // delete now diff --git a/process/process.go b/process/process.go index cf748a26..1ac3dcd5 100644 --- a/process/process.go +++ b/process/process.go @@ -59,9 +59,29 @@ type Process struct { // Profile returns the assigned layered profile. func (p *Process) Profile() *profile.LayeredProfile { + if p == nil { + return nil + } + return p.profile } +// GetLastSeen returns the unix timestamp when the process was last seen. +func (p *Process) GetLastSeen() int64 { + p.Lock() + defer p.Unlock() + + return p.LastSeen +} + +// SetLastSeen sets the unix timestamp when the process was last seen. +func (p *Process) SetLastSeen(lastSeen int64) { + p.Lock() + defer p.Unlock() + + p.LastSeen = lastSeen +} + // Strings returns a string representation of process. func (p *Process) String() string { if p == nil { diff --git a/profile/get.go b/profile/get.go index c8d49f25..1de5c049 100644 --- a/profile/get.go +++ b/profile/get.go @@ -97,7 +97,8 @@ func GetProfile(source profileSource, id, linkedPath string) ( // Process profiles coming directly from the database. // As we don't use any caching, these will be new objects. - // Mark the profile as being saved internally in order to bypass checks. + // Mark the profile as being saved internally in order to not trigger an + // update after saving it to the database. profile.internalSave = true // Add a layeredProfile to local profiles. diff --git a/profile/profile-layered.go b/profile/profile-layered.go index 906de324..b062528c 100644 --- a/profile/profile-layered.go +++ b/profile/profile-layered.go @@ -63,7 +63,6 @@ func NewLayeredProfile(localProfile *Profile) *LayeredProfile { localProfile: localProfile, layers: make([]*Profile, 0, len(localProfile.LinkedProfiles)+1), LayerIDs: make([]string, 0, len(localProfile.LinkedProfiles)+1), - RevisionCounter: 0, validityFlag: abool.NewBool(true), globalValidityFlag: config.NewValidityFlag(), securityLevel: &securityLevelVal, @@ -361,21 +360,26 @@ func (lp *LayeredProfile) wrapSecurityLevelOption(configKey string, globalConfig } func (lp *LayeredProfile) wrapBoolOption(configKey string, globalConfig config.BoolOption) config.BoolOption { - valid := no + revCnt := lp.RevisionCounter var value bool + var refreshLock sync.Mutex return func() bool { - if !valid.IsSet() { - valid = lp.getValidityFlag() + refreshLock.Lock() + defer refreshLock.Unlock() + // Check if we need to refresh the value. + if revCnt != lp.RevisionCounter { + revCnt = lp.RevisionCounter + + // Go through all layers to find an active value. found := false - layerLoop: for _, layer := range lp.layers { layerValue, ok := layer.configPerspective.GetAsBool(configKey) if ok { found = true value = layerValue - break layerLoop + break } } if !found { @@ -388,21 +392,26 @@ func (lp *LayeredProfile) wrapBoolOption(configKey string, globalConfig config.B } func (lp *LayeredProfile) wrapIntOption(configKey string, globalConfig config.IntOption) config.IntOption { - valid := no + revCnt := lp.RevisionCounter var value int64 + var refreshLock sync.Mutex return func() int64 { - if !valid.IsSet() { - valid = lp.getValidityFlag() + refreshLock.Lock() + defer refreshLock.Unlock() + // Check if we need to refresh the value. + if revCnt != lp.RevisionCounter { + revCnt = lp.RevisionCounter + + // Go through all layers to find an active value. found := false - layerLoop: for _, layer := range lp.layers { layerValue, ok := layer.configPerspective.GetAsInt(configKey) if ok { found = true value = layerValue - break layerLoop + break } } if !found { @@ -432,21 +441,26 @@ func (lp *LayeredProfile) GetProfileSource(configKey string) string { For later: func (lp *LayeredProfile) wrapStringOption(configKey string, globalConfig config.StringOption) config.StringOption { - valid := no + revCnt := lp.RevisionCounter var value string + var refreshLock sync.Mutex return func() string { - if !valid.IsSet() { - valid = lp.getValidityFlag() + refreshLock.Lock() + defer refreshLock.Unlock() + // Check if we need to refresh the value. + if revCnt != lp.RevisionCounter { + revCnt = lp.RevisionCounter + + // Go through all layers to find an active value. found := false - layerLoop: for _, layer := range lp.layers { layerValue, ok := layer.configPerspective.GetAsString(configKey) if ok { found = true value = layerValue - break layerLoop + break } } if !found {