From 2cbaf126e9e9e6b5e0c99c5aa2e4f02333a73708 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 15 Feb 2022 13:59:56 +0100 Subject: [PATCH 1/5] Remove deprecated MarkUsed functions --- process/profile.go | 5 +---- profile/profile-layered.go | 5 ----- profile/profile.go | 24 ------------------------ 3 files changed, 1 insertion(+), 33 deletions(-) diff --git a/process/profile.go b/process/profile.go index 381411fd..4a3a3865 100644 --- a/process/profile.go +++ b/process/profile.go @@ -105,11 +105,8 @@ func (p *Process) UpdateProfileMetadata() { // Update metadata of profile. metadataUpdated := localProfile.UpdateMetadata(p.Path) - // Mark profile as used. - profileChanged := localProfile.MarkUsed() - // Save the profile if we changed something. - if metadataUpdated || profileChanged { + if metadataUpdated { err := localProfile.Save() if err != nil { log.Warningf("process: failed to save profile %s: %s", localProfile.ScopedID(), err) diff --git a/profile/profile-layered.go b/profile/profile-layered.go index e3747245..b67e1bca 100644 --- a/profile/profile-layered.go +++ b/profile/profile-layered.go @@ -276,11 +276,6 @@ func (lp *LayeredProfile) updateCaches() { atomic.StoreUint32(lp.securityLevel, uint32(newLevel)) } -// MarkUsed marks the localProfile as used. -func (lp *LayeredProfile) MarkUsed() { - lp.localProfile.MarkUsed() -} - // SecurityLevel returns the highest security level of all layered profiles. This function is atomic and does not require any locking. func (lp *LayeredProfile) SecurityLevel() uint8 { return uint8(atomic.LoadUint32(lp.securityLevel)) diff --git a/profile/profile.go b/profile/profile.go index bb2d44c9..f23dafa2 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -281,30 +281,6 @@ func (profile *Profile) LastActive() int64 { return atomic.LoadInt64(profile.lastActive) } -// MarkUsed updates ApproxLastUsed when it's been a while and saves the profile if it was changed. -func (profile *Profile) MarkUsed() (changed bool) { - /* - TODO: - This might be one of the things causing problems with disappearing settings. - Possibly this is called with an outdated profile and then kills settings - already in the database. - Generally, it probably causes more harm than good if we periodically touch - the most important database entries just to update a timestamp. - We should save this data elsewhere and make configuration data as stable as - possible. - - profile.Lock() - defer profile.Unlock() - - if time.Now().Add(-lastUsedUpdateThreshold).Unix() > profile.ApproxLastUsed { - profile.ApproxLastUsed = time.Now().Unix() - return true - } - */ - - return false -} - // String returns a string representation of the Profile. func (profile *Profile) String() string { return fmt.Sprintf("<%s %s/%s>", profile.Name, profile.Source, profile.ID) From 888b33918a8946bf7e92d2419b4550536edf77b4 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 15 Feb 2022 14:35:28 +0100 Subject: [PATCH 2/5] Fix deleting profiles --- firewall/prompt.go | 2 +- process/find.go | 2 +- process/profile.go | 2 +- profile/database.go | 22 +++++++++------ profile/get.go | 58 +++++++++++++++++++++++++------------- profile/profile-layered.go | 4 +-- profile/profile.go | 25 ++++++++++++++++ profile/special.go | 6 ++++ 8 files changed, 89 insertions(+), 32 deletions(-) diff --git a/firewall/prompt.go b/firewall/prompt.go index 5b3b07a4..0356516a 100644 --- a/firewall/prompt.go +++ b/firewall/prompt.go @@ -259,7 +259,7 @@ func saveResponse(p *profile.Profile, entity *intel.Entity, promptResponse strin // Update the profile if necessary. if p.IsOutdated() { var err error - p, err = profile.GetProfile(p.Source, p.ID, p.LinkedPath) + p, err = profile.GetProfile(p.Source, p.ID, p.LinkedPath, false) if err != nil { return err } diff --git a/process/find.go b/process/find.go index 5ce75574..832c17a7 100644 --- a/process/find.go +++ b/process/find.go @@ -64,7 +64,7 @@ func GetNetworkHost(ctx context.Context, remoteIP net.IP) (process *Process, err } // Get the (linked) local profile. - networkHostProfile, err := profile.GetProfile(profile.SourceNetwork, remoteIP.String(), "") + networkHostProfile, err := profile.GetProfile(profile.SourceNetwork, remoteIP.String(), "", false) if err != nil { return nil, err } diff --git a/process/profile.go b/process/profile.go index 4a3a3865..d440756b 100644 --- a/process/profile.go +++ b/process/profile.go @@ -81,7 +81,7 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { } // Get the (linked) local profile. - localProfile, err := profile.GetProfile(profile.SourceLocal, profileID, p.Path) + localProfile, err := profile.GetProfile(profile.SourceLocal, profileID, p.Path, false) if err != nil { return false, err } diff --git a/profile/database.go b/profile/database.go index 3ddd9aea..709aaa5a 100644 --- a/profile/database.go +++ b/profile/database.go @@ -61,25 +61,31 @@ func startProfileUpdateChecker() error { // Get active profile. activeProfile := getActiveProfile(strings.TrimPrefix(r.Key(), profilesDBPath)) if activeProfile == nil { + // Don't do any additional actions if the profile is not active. continue profileFeed } - // If the record is being deleted, but there is an active profile, + // If the record is being deleted, reset the profile. // create an empty profile instead. if r.Meta().IsDeleted() { - newProfile := New( + newProfile, err := GetProfile( activeProfile.Source, activeProfile.ID, activeProfile.LinkedPath, - nil, + true, ) - // Copy some metadata from the old profile. - newProfile.Name = activeProfile.Name - // Save the new profile. - err := newProfile.Save() if err != nil { - log.Errorf("profile: failed to save new profile for profile reset: %s", err) + log.Errorf("profile: failed to create new profile after reset: %s", err) + } else { + // Copy metadata from the old profile. + newProfile.copyMetadataFrom(activeProfile) + // Save the new profile. + err := newProfile.Save() + if err != nil { + log.Errorf("profile: failed to save new profile after reset: %s", err) + } } + // Set to outdated, so it is loaded in the layered profiles. activeProfile.outdated.Set() } diff --git a/profile/get.go b/profile/get.go index de4c25e8..f2cb39b2 100644 --- a/profile/get.go +++ b/profile/get.go @@ -17,7 +17,7 @@ var getProfileLock sync.Mutex // linkedPath parameters whenever available. The linkedPath is used as the key // for locking concurrent requests, so it must be supplied if available. // If linkedPath is not supplied, source and id make up the key instead. -func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit +func GetProfile(source profileSource, id, linkedPath string, reset bool) ( //nolint:gocognit profile *Profile, err error, ) { @@ -40,28 +40,23 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit if profile != nil { profile.MarkStillActive() - if profile.outdated.IsSet() { + if profile.outdated.IsSet() || reset { previousVersion = profile } else { return profile, nil } } + // Get from database. - profile, err = getProfile(scopedID) - - // Check if the request is for a special profile that may need a reset. - if err == nil && specialProfileNeedsReset(profile) { - // Trigger creation of special profile. - err = database.ErrNotFound - } - - // If we cannot find a profile, check if the request is for a special - // profile we can create. - if errors.Is(err, database.ErrNotFound) { - profile = getSpecialProfile(id, linkedPath) - if profile != nil { - err = nil + if !reset { + profile, err = getProfile(scopedID) + // Check if the profile is special and needs a reset. + if err == nil && specialProfileNeedsReset(profile) { + profile = getSpecialProfile(id, linkedPath) } + } else { + // Simulate missing profile to create new one. + err = database.ErrNotFound } case linkedPath != "": @@ -70,23 +65,48 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit // the linked path. profile = findActiveProfile(linkedPath) if profile != nil { - if profile.outdated.IsSet() { + if profile.outdated.IsSet() || reset { previousVersion = profile } else { return profile, nil } } + // Get from database. - profile, err = findProfile(linkedPath) + if !reset { + profile, err = findProfile(linkedPath) + // Check if the profile is special and needs a reset. + if err == nil && specialProfileNeedsReset(profile) { + profile = getSpecialProfile(id, linkedPath) + } + } else { + // Simulate missing profile to create new one. + err = database.ErrNotFound + } default: return nil, errors.New("cannot fetch profile without ID or path") } + + // Create new profile if none was found. + if errors.Is(err, database.ErrNotFound) { + err = nil + + // Check if there is a special profile for this ID. + profile = getSpecialProfile(id, linkedPath) + + // If not, create a standard profile. + if profile == nil { + profile = New(SourceLocal, id, linkedPath, nil) + } + } + + // If there was a non-recoverable error, return here. if err != nil { return nil, err } - // Process profiles coming directly from the database. + // Process profiles are coming directly from the database or are new. // As we don't use any caching, these will be new objects. // Add a layeredProfile to local and network profiles. diff --git a/profile/profile-layered.go b/profile/profile-layered.go index b67e1bca..ffb77bd2 100644 --- a/profile/profile-layered.go +++ b/profile/profile-layered.go @@ -239,9 +239,9 @@ func (lp *LayeredProfile) Update() (revisionCounter uint64) { if layer.outdated.IsSet() { changed = true // update layer - newLayer, err := GetProfile(layer.Source, layer.ID, layer.LinkedPath) + newLayer, err := GetProfile(layer.Source, layer.ID, layer.LinkedPath, false) if err != nil { - log.Errorf("profiles: failed to update profile %s", layer.ScopedID()) + log.Errorf("profiles: failed to update profile %s: %s", layer.ScopedID(), err) } else { lp.layers[i] = newLayer } diff --git a/profile/profile.go b/profile/profile.go index f23dafa2..dd7ae25f 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -445,6 +445,31 @@ func (profile *Profile) UpdateMetadata(binaryPath string) (changed bool) { return changed } +func (profile *Profile) copyMetadataFrom(otherProfile *Profile) (changed bool) { + if profile.Name != otherProfile.Name { + profile.Name = otherProfile.Name + changed = true + } + if profile.Description != otherProfile.Description { + profile.Description = otherProfile.Description + changed = true + } + if profile.Homepage != otherProfile.Homepage { + profile.Homepage = otherProfile.Homepage + changed = true + } + if profile.Icon != otherProfile.Icon { + profile.Icon = otherProfile.Icon + changed = true + } + if profile.IconType != otherProfile.IconType { + profile.IconType = otherProfile.IconType + changed = true + } + + return +} + // updateMetadataFromSystem updates the profile metadata with data from the // operating system and saves it afterwards. func (profile *Profile) updateMetadataFromSystem(ctx context.Context) error { diff --git a/profile/special.go b/profile/special.go index cd04cc0d..13e9b5a9 100644 --- a/profile/special.go +++ b/profile/special.go @@ -36,6 +36,8 @@ In order to respect the app settings of the actual application, DNS requests fro - Outgoing Rules (without global rules) - Block Bypassing - Filter Lists + +If you think you might have messed up the settings of the System DNS Client, just delete the profile below to reset it to the defaults. ` // PortmasterProfileID is the profile ID used for the Portmaster Core itself. @@ -193,6 +195,10 @@ func getSpecialProfile(profileID, linkedPath string) *Profile { // check if the special profile has not been changed by the user and if not, // check if the profile is outdated and can be upgraded. func specialProfileNeedsReset(profile *Profile) bool { + if profile == nil { + return false + } + switch { case profile.Source != SourceLocal: // Special profiles live in the local scope only. From 3be1c78e163a8a91e187f537668d3f3775c864f9 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 15 Feb 2022 14:35:41 +0100 Subject: [PATCH 3/5] Fix profile config parsing --- profile/database.go | 8 ++------ profile/get.go | 7 ++----- profile/profile.go | 23 ++++++++++------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/profile/database.go b/profile/database.go index 709aaa5a..07250356 100644 --- a/profile/database.go +++ b/profile/database.go @@ -131,12 +131,8 @@ func (h *databaseHook) PrePut(r record.Record) (record.Record, error) { // clean config config.CleanHierarchicalConfig(profile.Config) - // prepare config - err = profile.prepConfig() - if err != nil { - // error here, warning when loading - return nil, err - } + // prepare profile + profile.prepProfile() // parse config err = profile.parseConfig() diff --git a/profile/get.go b/profile/get.go index f2cb39b2..9d49b772 100644 --- a/profile/get.go +++ b/profile/get.go @@ -178,11 +178,8 @@ func prepProfile(r record.Record) (*Profile, error) { return nil, err } - // prepare config - err = profile.prepConfig() - if err != nil { - log.Errorf("profiles: profile %s has (partly) invalid configuration: %s", profile.ID, err) - } + // prepare profile + profile.prepProfile() // parse config err = profile.parseConfig() diff --git a/profile/profile.go b/profile/profile.go index dd7ae25f..1e126714 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -137,28 +137,27 @@ type Profile struct { //nolint:maligned // not worth the effort savedInternally bool } -func (profile *Profile) prepConfig() (err error) { +func (profile *Profile) prepProfile() { // prepare configuration - profile.configPerspective, err = config.NewPerspective(profile.Config) profile.outdated = abool.New() profile.lastActive = new(int64) - return } func (profile *Profile) parseConfig() error { - if profile.configPerspective == nil { - return errors.New("config not prepared") - } - - // check if already parsed + // Check if already parsed. if profile.dataParsed { return nil } + + // Create new perspective and marked as parsed. + var err error + profile.configPerspective, err = config.NewPerspective(profile.Config) + if err != nil { + return fmt.Errorf("failed to create config perspective: %w", err) + } profile.dataParsed = true - var err error var lastErr error - action, ok := profile.configPerspective.GetAsString(CfgOptionDefaultActionKey) profile.defaultAction = DefaultActionNotSet if ok { @@ -238,9 +237,7 @@ func New( profile.makeKey() // Prepare and parse initial profile config. - if err := profile.prepConfig(); err != nil { - log.Errorf("profile: failed to prep new profile: %s", err) - } + profile.prepProfile() if err := profile.parseConfig(); err != nil { log.Errorf("profile: failed to parse new profile: %s", err) } From d491e511277c7bae14d963cce871ecc9556cfa4e Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 15 Feb 2022 16:07:29 +0100 Subject: [PATCH 4/5] Improve handling of layered profile on profile update --- profile/database.go | 7 +++++-- profile/get.go | 5 ++--- profile/profile-layered.go | 8 +++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/profile/database.go b/profile/database.go index 07250356..53be3405 100644 --- a/profile/database.go +++ b/profile/database.go @@ -80,14 +80,17 @@ func startProfileUpdateChecker() error { // Copy metadata from the old profile. newProfile.copyMetadataFrom(activeProfile) // Save the new profile. - err := newProfile.Save() + err = newProfile.Save() if err != nil { log.Errorf("profile: failed to save new profile after reset: %s", err) } } - // Set to outdated, so it is loaded in the layered profiles. + // If the new profile was successfully created, update layered profile. activeProfile.outdated.Set() + if err == nil { + newProfile.layeredProfile.Update() + } } // Always increase the revision counter of the layer profile. diff --git a/profile/get.go b/profile/get.go index 9d49b772..c6ef0b5d 100644 --- a/profile/get.go +++ b/profile/get.go @@ -14,9 +14,7 @@ var getProfileLock sync.Mutex // GetProfile fetches a profile. This function ensures that the loaded profile // is shared among all callers. You must always supply both the scopedID and -// linkedPath parameters whenever available. The linkedPath is used as the key -// for locking concurrent requests, so it must be supplied if available. -// If linkedPath is not supplied, source and id make up the key instead. +// linkedPath parameters whenever available. func GetProfile(source profileSource, id, linkedPath string, reset bool) ( //nolint:gocognit profile *Profile, err error, @@ -112,6 +110,7 @@ func GetProfile(source profileSource, id, linkedPath string, reset bool) ( //nol // Add a layeredProfile to local and network profiles. if profile.Source == SourceLocal || profile.Source == SourceNetwork { // If we are refetching, assign the layered profile from the previous version. + // The internal references will be updated when the layered profile checks for updates. if previousVersion != nil { profile.layeredProfile = previousVersion.layeredProfile } diff --git a/profile/profile-layered.go b/profile/profile-layered.go index ffb77bd2..bf965944 100644 --- a/profile/profile-layered.go +++ b/profile/profile-layered.go @@ -238,13 +238,19 @@ func (lp *LayeredProfile) Update() (revisionCounter uint64) { for i, layer := range lp.layers { if layer.outdated.IsSet() { changed = true - // update layer + + // Update layer. newLayer, err := GetProfile(layer.Source, layer.ID, layer.LinkedPath, false) if err != nil { log.Errorf("profiles: failed to update profile %s: %s", layer.ScopedID(), err) } else { lp.layers[i] = newLayer } + + // Update local profile reference. + if i == 0 { + lp.localProfile = newLayer + } } } if !lp.globalValidityFlag.IsValid() { From 1b98dd9d3915e85617b183182b6653a7b9805c4d Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 15 Feb 2022 16:07:44 +0100 Subject: [PATCH 5/5] Add setting category to network rating setting --- status/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/status/config.go b/status/config.go index 4e626993..11daf942 100644 --- a/status/config.go +++ b/status/config.go @@ -19,6 +19,7 @@ func registerConfig() error { DefaultValue: false, Annotations: config.Annotations{ config.DisplayOrderAnnotation: 514, + config.CategoryAnnotation: "User Interface", }, }); err != nil { return err