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 381411fd..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 } @@ -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/database.go b/profile/database.go index 3ddd9aea..53be3405 100644 --- a/profile/database.go +++ b/profile/database.go @@ -61,27 +61,36 @@ 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. + + // 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. @@ -125,12 +134,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 de4c25e8..c6ef0b5d 100644 --- a/profile/get.go +++ b/profile/get.go @@ -14,10 +14,8 @@ 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. -func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit +// linkedPath parameters whenever available. +func GetProfile(source profileSource, id, linkedPath string, reset bool) ( //nolint:gocognit profile *Profile, err error, ) { @@ -40,28 +38,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,28 +63,54 @@ 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. 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 } @@ -158,11 +177,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-layered.go b/profile/profile-layered.go index e3747245..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 - newLayer, err := GetProfile(layer.Source, layer.ID, layer.LinkedPath) + + // Update layer. + 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 } + + // Update local profile reference. + if i == 0 { + lp.localProfile = newLayer + } } } if !lp.globalValidityFlag.IsValid() { @@ -276,11 +282,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..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) } @@ -281,30 +278,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) @@ -469,6 +442,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. 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