From 595f4c0106cf283dc0d5ab1d3ceb70a9c1044904 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 6 Oct 2022 10:33:25 +0200 Subject: [PATCH] Improve profile metadata handling --- process/find.go | 16 +---- process/profile.go | 24 ------- process/tags/appimage_unix.go | 7 +- process/tags/svchost_windows.go | 37 +++++----- process/tags/winstore_windows.go | 15 ++-- profile/fingerprint.go | 5 +- profile/get.go | 34 +++++++-- profile/profile.go | 117 ++++++++++++++++--------------- profile/special.go | 47 ++++++++++--- 9 files changed, 165 insertions(+), 137 deletions(-) diff --git a/process/find.go b/process/find.go index 9f021426..35ff4b75 100644 --- a/process/find.go +++ b/process/find.go @@ -64,14 +64,14 @@ func GetProcessByConnection(ctx context.Context, pktInfo *packet.Info) (process func GetNetworkHost(ctx context.Context, remoteIP net.IP) (process *Process, err error) { //nolint:interfacer now := time.Now().Unix() networkHost := &Process{ - Name: fmt.Sprintf("Network Host %s", remoteIP), - UserName: "Unknown", + Name: fmt.Sprintf("Device at %s", remoteIP), + UserName: "N/A", UserID: NetworkHostProcessID, Pid: NetworkHostProcessID, ParentPid: NetworkHostProcessID, Tags: []profile.Tag{ { - Key: "net", + Key: "ip", Value: remoteIP.String(), }, }, @@ -89,16 +89,6 @@ func GetNetworkHost(ctx context.Context, remoteIP net.IP) (process *Process, err networkHost.PrimaryProfileID = networkHostProfile.ScopedID() networkHost.profile = networkHostProfile.LayeredProfile() - if networkHostProfile.Name == "" { - // Assign name and save. - networkHostProfile.Name = networkHost.Name - - err := networkHostProfile.Save() - if err != nil { - log.Warningf("process: failed to save profile %s: %s", networkHostProfile.ScopedID(), err) - } - } - return networkHost, nil } diff --git a/process/profile.go b/process/profile.go index aab86996..9c49e832 100644 --- a/process/profile.go +++ b/process/profile.go @@ -15,9 +15,6 @@ var ownPID = os.Getpid() // GetProfile finds and assigns a profile set to the process. func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { - // Update profile metadata outside of *Process lock. - defer p.UpdateProfileMetadata() - p.Lock() defer p.Unlock() @@ -114,24 +111,3 @@ func (p *Process) loadSpecialProfile(_ context.Context) (*profile.Profile, error // Return special profile. return profile.GetSpecialProfile(specialProfileID, p.Path) } - -// UpdateProfileMetadata updates the metadata of the local profile -// as required. -func (p *Process) UpdateProfileMetadata() { - // Check if there is a profile to work with. - localProfile := p.Profile().LocalProfile() - if localProfile == nil { - return - } - - // Update metadata of profile. - metadataUpdated := localProfile.UpdateMetadata(p.Path) - - // Save the profile if we changed something. - if metadataUpdated { - err := localProfile.Save() - if err != nil { - log.Warningf("process: failed to save profile %s: %s", localProfile.ScopedID(), err) - } - } -} diff --git a/process/tags/appimage_unix.go b/process/tags/appimage_unix.go index 54e9d8bb..094a5f39 100644 --- a/process/tags/appimage_unix.go +++ b/process/tags/appimage_unix.go @@ -3,6 +3,7 @@ package tags import ( "strings" + "github.com/safing/portbase/utils/osdetail" "github.com/safing/portmaster/process" "github.com/safing/portmaster/profile" ) @@ -71,8 +72,10 @@ func (h *AppImageHandler) CreateProfile(p *process.Process) *profile.Profile { for _, tag := range p.Tags { if tag.Key == appImagePathTagKey { return profile.New(&profile.Profile{ - Source: profile.SourceLocal, - PresentationPath: p.Path, + Source: profile.SourceLocal, + Name: osdetail.GenerateBinaryNameFromPath(tag.Value), + PresentationPath: p.Path, + UsePresentationPath: true, Fingerprints: []profile.Fingerprint{ { Type: profile.FingerprintTypePathID, diff --git a/process/tags/svchost_windows.go b/process/tags/svchost_windows.go index 724c29a8..60ab951b 100644 --- a/process/tags/svchost_windows.go +++ b/process/tags/svchost_windows.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/safing/portbase/log" + "github.com/safing/portbase/utils/osdetail" "github.com/safing/portmaster/process" "github.com/safing/portmaster/profile" ) @@ -17,7 +18,7 @@ func init() { } const ( - svchostName = "SvcHost" + svchostName = "Service Host" svchostTagKey = "svchost" ) @@ -26,7 +27,7 @@ type SVCHostTagHandler struct{} // Name returns the tag handler name. func (h *SVCHostTagHandler) Name() string { - return svcHostName + return svchostName } // TagDescriptions returns a list of all possible tags and their description @@ -34,7 +35,7 @@ func (h *SVCHostTagHandler) Name() string { func (h *SVCHostTagHandler) TagDescriptions() []process.TagDescription { return []process.TagDescription{ process.TagDescription{ - ID: svcHostTagKey, + ID: svchostTagKey, Name: "SvcHost Service Name", Description: "Name of a service running in svchost.exe as reported by Windows.", }, @@ -43,7 +44,7 @@ func (h *SVCHostTagHandler) TagDescriptions() []process.TagDescription { // TagKeys returns a list of all possible tag keys of this handler. func (h *SVCHostTagHandler) TagKeys() []string { - return []string{svcHostTagKey} + return []string{svchostTagKey} } // AddTags adds tags to the given process. @@ -61,7 +62,7 @@ func (h *SVCHostTagHandler) AddTags(p *process.Process) { p.Name += fmt.Sprintf(" (%s)", strings.Join(svcNames, ", ")) // Add services as tags. for _, svcName := range svcNames { - p.Tag = append(p.Tag, profile.Tag{ + p.Tags = append(p.Tags, profile.Tag{ Key: svchostTagKey, Value: svcName, }) @@ -78,19 +79,19 @@ func (h *SVCHostTagHandler) AddTags(p *process.Process) { func (h *SVCHostTagHandler) CreateProfile(p *process.Process) *profile.Profile { for _, tag := range p.Tags { if tag.Key == svchostTagKey { - return profile.New( - profile.SourceLocal, - "", - "Windows Service: "+tag.Value, - p.Path, - []profile.Fingerprint{profile.Fingerprint{ - Type: profile.FingerprintTypeTagID, - Key: tag.Key, - Operation: profile.FingerprintOperationEqualsID, - Value: tag.Value, - }}, - nil, - ) + return profile.New(&profile.Profile{ + Source: profile.SourceLocal, + Name: "Windows Service: " + osdetail.GenerateBinaryNameFromPath(tag.Value), + UsePresentationPath: false, + Fingerprints: []profile.Fingerprint{ + profile.Fingerprint{ + Type: profile.FingerprintTypeTagID, + Key: tag.Key, + Operation: profile.FingerprintOperationEqualsID, + Value: tag.Value, + }, + }, + }) } } return nil diff --git a/process/tags/winstore_windows.go b/process/tags/winstore_windows.go index 86cf1563..bc3b538d 100644 --- a/process/tags/winstore_windows.go +++ b/process/tags/winstore_windows.go @@ -4,6 +4,8 @@ import ( "os" "strings" + "github.com/safing/portbase/utils/osdetail" + "github.com/safing/portbase/log" "github.com/safing/portbase/utils" @@ -20,7 +22,7 @@ func init() { // Add custom WindowsApps path. customWinStorePath := os.ExpandEnv(`%ProgramFiles%\WindowsApps\`) - if !utils.StringSliceEqual(winStorePaths, customWinStorePath) { + if !utils.StringInSlice(winStorePaths, customWinStorePath) { winStorePaths = append(winStorePaths, customWinStorePath) } } @@ -64,7 +66,7 @@ func (h *WinStoreHandler) AddTags(p *process.Process) { var appDir string for _, winStorePath := range winStorePaths { if strings.HasPrefix(p.Path, winStorePath) { - appDir := strings.SplitN(strings.TrimPrefix(p.Path, winStorePath), `\`, 2)[0] + appDir = strings.SplitN(strings.TrimPrefix(p.Path, winStorePath), `\`, 2)[0] break } } @@ -75,7 +77,7 @@ func (h *WinStoreHandler) AddTags(p *process.Process) { // Extract information from path. // Example: Microsoft.Office.OneNote_17.6769.57631.0_x64__8wekyb3d8bbwe splitted := strings.Split(appDir, "_") - if splitted != 5 { // Four fields, one "__". + if len(splitted) != 5 { // Four fields, one "__". log.Debugf("profile/tags: windows store app has incompatible app dir format: %q", appDir) return } @@ -102,9 +104,10 @@ func (h *WinStoreHandler) CreateProfile(p *process.Process) *profile.Profile { for _, tag := range p.Tags { if tag.Key == winStoreAppNameTagKey { return profile.New(&profile.Profile{ - Source: profile.SourceLocal, - Name: tag.Value, - PresentationPath: p.Path, + Source: profile.SourceLocal, + Name: osdetail.GenerateBinaryNameFromPath(tag.Value), + PresentationPath: p.Path, + UsePresentationPath: true, Fingerprints: []profile.Fingerprint{ { Type: profile.FingerprintTypeTagID, diff --git a/profile/fingerprint.go b/profile/fingerprint.go index 4b71cddf..b0177308 100644 --- a/profile/fingerprint.go +++ b/profile/fingerprint.go @@ -163,8 +163,9 @@ type parsedFingerprints struct { func parseFingerprints(raw []Fingerprint, deprecatedLinkedPath string) (parsed *parsedFingerprints, firstErr error) { parsed = &parsedFingerprints{} - // Add deprecated linked path to fingerprints. - if deprecatedLinkedPath != "" { + // Add deprecated LinkedPath to fingerprints, if they are empty. + // TODO: Remove in v1.5 + if len(raw) == 0 && deprecatedLinkedPath != "" { parsed.pathPrints = append(parsed.pathPrints, &fingerprintEquals{ Fingerprint: Fingerprint{ Type: FingerprintTypePathID, diff --git a/profile/get.go b/profile/get.go index d9820c18..ad8d018b 100644 --- a/profile/get.go +++ b/profile/get.go @@ -70,7 +70,10 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P } // If we still don't have a profile, create a new one. + var created bool if profile == nil { + created = true + // Try the profile creation callback, if we have one. if createProfileCallback != nil { profile = createProfileCallback() @@ -84,9 +87,10 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P } profile = New(&Profile{ - ID: id, - Source: SourceLocal, - PresentationPath: md.Path(), + ID: id, + Source: SourceLocal, + PresentationPath: md.Path(), + UsePresentationPath: true, Fingerprints: []Fingerprint{ { Type: FingerprintTypePathID, @@ -98,6 +102,25 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P } } + // Initialize and update profile. + + // Update metadata. + changed := profile.updateMetadata(md.Path()) + + // Save if created or changed. + if created || changed { + // Save profile. + err := profile.Save() + if err != nil { + log.Warningf("profile: failed to save profile %s after creation: %s", profile.ScopedID(), err) + } + } + + // Trigger further metadata fetching from system if profile was created. + if created && profile.UsePresentationPath { + module.StartWorker("get profile metadata", profile.updateMetadataFromSystem) + } + // Prepare profile for first use. // Process profiles are coming directly from the database or are new. @@ -158,7 +181,10 @@ profileFeed: prints, err := loadProfileFingerprints(r) if err != nil { log.Debugf("profile: failed to load fingerprints of %s: %s", r.Key(), err) - continue + } + // Continue with any returned fingerprints. + if prints == nil { + continue profileFeed } // Get matching score and compare. diff --git a/profile/profile.go b/profile/profile.go index 09dc8d68..fb56d90c 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "path/filepath" "strings" "sync" "sync/atomic" @@ -89,6 +88,10 @@ type Profile struct { //nolint:maligned // not worth the effort // Is automatically removed when the path does not exist. // Is automatically populated with the next match when empty. PresentationPath string + // UsePresentationPath can be used to enable/disable fetching information + // from the executable at PresentationPath. In some cases, this is not + // desirable. + UsePresentationPath bool // Fingerprints holds process matching information. Fingerprints []Fingerprint // SecurityLevel is the mininum security level to apply to @@ -419,48 +422,54 @@ func EnsureProfile(r record.Record) (*Profile, error) { return newProfile, nil } -// UpdateMetadata updates meta data fields on the profile and returns whether -// the profile was changed. If there is data that needs to be fetched from the -// operating system, it will start an async worker to fetch that data and save -// the profile afterwards. -func (profile *Profile) UpdateMetadata(binaryPath string) (changed bool) { +// updateMetadata updates meta data fields on the profile and returns whether +// the profile was changed. +func (profile *Profile) updateMetadata(binaryPath string) (changed bool) { // Check if this is a local profile, else warn and return. if profile.Source != SourceLocal { log.Warningf("tried to update metadata for non-local profile %s", profile.ScopedID()) return false } - profile.Lock() - defer profile.Unlock() - - // Update special profile and return if it was one. - if ok, changed := updateSpecialProfileMetadata(profile, binaryPath); ok { - return changed + // Set PresentationPath if unset. + if profile.PresentationPath == "" && binaryPath != "" { + profile.PresentationPath = binaryPath + changed = true } - var needsUpdateFromSystem bool + // Migrate LinkedPath to PresentationPath. + // TODO: Remove in v1.5 + if profile.PresentationPath == "" && profile.LinkedPath != "" { + profile.PresentationPath = profile.LinkedPath + changed = true + } - // Check profile name. - filename := filepath.Base(profile.PresentationPath) - - // Update profile name if it is empty or equals the filename, which is the - // case for older profiles. - if strings.TrimSpace(profile.Name) == "" || profile.Name == filename { - // Generate a default profile name if does not exist. + // Set Name if unset. + if profile.Name == "" && profile.PresentationPath != "" { + // Generate a default profile name from path. profile.Name = osdetail.GenerateBinaryNameFromPath(profile.PresentationPath) - if profile.Name == filename { - // TODO: Theoretically, the generated name could be identical to the - // filename. - // As a quick fix, append a space to the name. - profile.Name += " " + changed = true + } + + // Migrato to Fingerprints. + // TODO: Remove in v1.5 + if len(profile.Fingerprints) == 0 && profile.LinkedPath != "" { + profile.Fingerprints = []Fingerprint{ + { + Type: FingerprintTypePathID, + Operation: FingerprintOperationEqualsID, + Value: profile.LinkedPath, + }, } changed = true - needsUpdateFromSystem = true } - // If needed, get more/better data from the operating system. - if needsUpdateFromSystem { - module.StartWorker("get profile metadata", profile.updateMetadataFromSystem) + // UI Backward Compatibility: + // Fill LinkedPath with PresentationPath + // TODO: Remove in v1.1 + if profile.LinkedPath == "" && profile.PresentationPath != "" { + profile.LinkedPath = profile.PresentationPath + changed = true } return changed @@ -469,23 +478,14 @@ func (profile *Profile) UpdateMetadata(binaryPath string) (changed bool) { // updateMetadataFromSystem updates the profile metadata with data from the // operating system and saves it afterwards. func (profile *Profile) updateMetadataFromSystem(ctx context.Context) error { + var changed bool + // This function is only valid for local profiles. if profile.Source != SourceLocal || profile.PresentationPath == "" { return fmt.Errorf("tried to update metadata for non-local or non-path profile %s", profile.ScopedID()) } - // Save the profile when finished, if needed. - save := false - defer func() { - if save { - err := profile.Save() - if err != nil { - log.Warningf("profile: failed to save %s after metadata update: %s", profile.ScopedID(), err) - } - } - }() - - // Get binary name from linked path. + // Get binary name from PresentationPath. newName, err := osdetail.GetBinaryNameFromSystem(profile.PresentationPath) if err != nil { switch { @@ -503,25 +503,26 @@ func (profile *Profile) updateMetadataFromSystem(ctx context.Context) error { return nil } - // Get filename of linked path for comparison. - filename := filepath.Base(profile.PresentationPath) + // Apply new data to profile. + func() { + // Lock profile for applying metadata. + profile.Lock() + defer profile.Unlock() - // TODO: Theoretically, the generated name from the system could be identical - // to the filename. This would mean that the worker is triggered every time - // the profile is freshly loaded. - if newName == filename { - // As a quick fix, append a space to the name. - newName += " " - } + // Apply new name if it changed. + if profile.Name != newName { + profile.Name = newName + changed = true + } + }() - // Lock profile for applying metadata. - profile.Lock() - defer profile.Unlock() - - // Apply new name if it changed. - if profile.Name != newName { - profile.Name = newName - save = true + // If anything changed, save the profile. + // profile.Lock must not be held! + if changed { + err := profile.Save() + if err != nil { + log.Warningf("profile: failed to save %s after metadata update: %s", profile.ScopedID(), err) + } } return nil diff --git a/profile/special.go b/profile/special.go index 89b06f3c..79c2a7b5 100644 --- a/profile/special.go +++ b/profile/special.go @@ -110,20 +110,40 @@ func GetSpecialProfile(id string, path string) ( //nolint:gocognit } // Get special profile from DB and check if it needs a reset. + var created bool profile, err = getProfile(scopedID) - if err != nil { - if !errors.Is(err, database.ErrNotFound) { - log.Warningf("profile: failed to get special profile %s: %s", id, err) + switch { + case err == nil: + // Reset profile if needed. + if specialProfileNeedsReset(profile) { + profile = createSpecialProfile(id, path) + created = true } + case !errors.Is(err, database.ErrNotFound): + // Warn when fetching from DB fails, and create new profile as fallback. + log.Warningf("profile: failed to get special profile %s: %s", id, err) + fallthrough + default: + // Create new profile if it does not exist (or failed to load). profile = createSpecialProfile(id, path) - } else if specialProfileNeedsReset(profile) { - log.Debugf("profile: resetting special profile %s", id) - profile = createSpecialProfile(id, path) + created = true } + // Check if creating the special profile was successful. if profile == nil { return nil, errors.New("given ID is not a special profile ID") } + // Update metadata + changed := updateSpecialProfileMetadata(profile, path) + + // Save if created or changed. + if created || changed { + err := profile.Save() + if err != nil { + log.Warningf("profile: failed to save special profile %s: %s", scopedID, err) + } + } + // Prepare profile for first use. // If we are refetching, assign the layered profile from the previous version. @@ -144,7 +164,7 @@ func GetSpecialProfile(id string, path string) ( //nolint:gocognit return profile, nil } -func updateSpecialProfileMetadata(profile *Profile, binaryPath string) (ok, changed bool) { +func updateSpecialProfileMetadata(profile *Profile, binaryPath string) (changed bool) { // Get new profile name and check if profile is applicable to special handling. var newProfileName, newDescription string switch profile.ID { @@ -170,7 +190,7 @@ func updateSpecialProfileMetadata(profile *Profile, binaryPath string) (ok, chan newProfileName = PortmasterNotifierProfileName newDescription = PortmasterNotifierProfileDescription default: - return false, false + return false } // Update profile name if needed. @@ -191,7 +211,7 @@ func updateSpecialProfileMetadata(profile *Profile, binaryPath string) (ok, chan changed = true } - return true, changed + return changed } func createSpecialProfile(profileID string, path string) *Profile { @@ -203,6 +223,13 @@ func createSpecialProfile(profileID string, path string) *Profile { PresentationPath: path, }) + case UnsolicitedProfileID: + return New(&Profile{ + ID: UnsolicitedProfileID, + Source: SourceLocal, + PresentationPath: path, + }) + case SystemProfileID: return New(&Profile{ ID: SystemProfileID, @@ -306,7 +333,7 @@ func specialProfileNeedsReset(profile *Profile) bool { switch profile.ID { case SystemResolverProfileID: - return canBeUpgraded(profile, "20.11.2021") + return canBeUpgraded(profile, "21.10.2022") case PortmasterAppProfileID: return canBeUpgraded(profile, "8.9.2021") default: