From 484012712f79f0a927aa14e07bd605e8779d9823 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 17 Nov 2020 09:33:28 +0100 Subject: [PATCH 1/2] Adapt profiles to use new binary metadata system --- firewall/prompt.go | 11 ++-- process/process.go | 76 ------------------------ process/profile.go | 15 ++--- profile/config-update.go | 2 +- profile/get.go | 26 ++++----- profile/module_test.go | 8 +-- profile/profile-layered.go | 2 +- profile/profile.go | 116 +++++++++++++++++++++++++++++++++++-- 8 files changed, 138 insertions(+), 118 deletions(-) diff --git a/firewall/prompt.go b/firewall/prompt.go index 5a723174..9a1d2787 100644 --- a/firewall/prompt.go +++ b/firewall/prompt.go @@ -152,10 +152,13 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack ) }) + // Get name of profile for notification. The profile is read-locked by the firewall handler. + profileName := localProfile.Name + // add message and actions switch { case conn.Inbound: - 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.Message = fmt.Sprintf("%s wants to accept connections from %s (%d/%d)", profileName, conn.Entity.IP.String(), conn.Entity.Protocol, conn.Entity.Port) n.AvailableActions = []*notifications.Action{ { ID: allowServingIP, @@ -167,7 +170,7 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack }, } 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.Message = fmt.Sprintf("%s wants to connect to %s (%d/%d)", profileName, conn.Entity.IP.String(), conn.Entity.Protocol, conn.Entity.Port) n.AvailableActions = []*notifications.Action{ { ID: allowIP, @@ -179,7 +182,7 @@ func createPrompt(ctx context.Context, conn *network.Connection, pkt packet.Pack }, } default: // connection to domain - n.Message = fmt.Sprintf("Application %s wants to connect to %s", conn.Process(), conn.Entity.Domain) + n.Message = fmt.Sprintf("%s wants to connect to %s", profileName, conn.Entity.Domain) n.AvailableActions = []*notifications.Action{ { ID: allowDomainAll, @@ -206,7 +209,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) if err != nil { return err } diff --git a/process/process.go b/process/process.go index 1ac3dcd5..9d932555 100644 --- a/process/process.go +++ b/process/process.go @@ -313,82 +313,6 @@ func loadProcess(ctx context.Context, pid int) (*Process, error) { // OS specifics new.specialOSInit() - - // TODO: App Icon - // new.Icon, err = - - // get Profile - // processPath := new.Path - // var applyProfile *profiles.Profile - // iterations := 0 - // for applyProfile == nil { - // - // iterations++ - // if iterations > 10 { - // log.Warningf("process: got into loop while getting profile for %s", new) - // break - // } - // - // applyProfile, err = profiles.GetActiveProfileByPath(processPath) - // if err == database.ErrNotFound { - // applyProfile, err = profiles.FindProfileByPath(processPath, new.UserHome) - // } - // if err != nil { - // log.Warningf("process: could not get profile for %s: %s", new, err) - // } else if applyProfile == nil { - // log.Warningf("process: no default profile found for %s", new) - // } else { - // - // // TODO: there is a lot of undefined behaviour if chaining framework profiles - // - // // process framework - // if applyProfile.Framework != nil { - // if applyProfile.Framework.FindParent > 0 { - // var ppid int32 - // for i := uint8(1); i < applyProfile.Framework.FindParent; i++ { - // parent, err := pInfo.Parent() - // if err != nil { - // return nil, err - // } - // ppid = parent.Pid - // } - // if applyProfile.Framework.MergeWithParent { - // return GetOrFindProcess(int(ppid)) - // } - // // processPath, err = os.Readlink(fmt.Sprintf("/proc/%d/exe", pid)) - // // if err != nil { - // // return nil, fmt.Errorf("could not read /proc/%d/exe: %s", pid, err) - // // } - // continue - // } - // - // newCommand, err := applyProfile.Framework.GetNewPath(new.CmdLine, new.Cwd) - // if err != nil { - // return nil, err - // } - // - // // assign - // new.CmdLine = newCommand - // new.Path = strings.SplitN(newCommand, " ", 2)[0] - // processPath = new.Path - // - // // make sure we loop - // applyProfile = nil - // continue - // } - // - // // apply profile to process - // log.Debugf("process: applied profile to %s: %s", new, applyProfile) - // new.Profile = applyProfile - // new.ProfileKey = applyProfile.GetKey().String() - // - // // update Profile with Process icon if Profile does not have one - // if !new.Profile.Default && new.Icon != "" && new.Profile.Icon == "" { - // new.Profile.Icon = new.Icon - // new.Profile.Save() - // } - // } - // } } new.Save() diff --git a/process/profile.go b/process/profile.go index bab71aa5..542cfc83 100644 --- a/process/profile.go +++ b/process/profile.go @@ -31,26 +31,19 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { } // Get the (linked) local profile. - localProfile, new, err := profile.GetProfile(profile.SourceLocal, profileID, p.Path) + localProfile, err := profile.GetProfile(profile.SourceLocal, profileID, p.Path) if err != nil { return false, err } - // If the local profile is new, add some information from the process. - if new { - localProfile.Name = p.ExecName - - // Special profiles will only have a name, but not an ExecName. - if localProfile.Name == "" { - localProfile.Name = p.Name - } - } + // Update metadata of profile. + metadataUpdated := localProfile.UpdateMetadata(p.Name) // Mark profile as used. profileChanged := localProfile.MarkUsed() // Save the profile if we changed something. - if new || profileChanged { + if metadataUpdated || profileChanged { err := localProfile.Save() if err != nil { log.Warningf("process: failed to save profile %s: %s", localProfile.ScopedID(), err) diff --git a/profile/config-update.go b/profile/config-update.go index 23ccaf59..22b76c60 100644 --- a/profile/config-update.go +++ b/profile/config-update.go @@ -76,7 +76,7 @@ func updateGlobalConfigProfile(ctx context.Context, task *modules.Task) error { } // build global profile for reference - profile := New(SourceSpecial, "global-config") + profile := New(SourceSpecial, "global-config", "") profile.Name = "Global Configuration" profile.Internal = true diff --git a/profile/get.go b/profile/get.go index 587f585a..dea8ba58 100644 --- a/profile/get.go +++ b/profile/get.go @@ -30,7 +30,6 @@ var getProfileSingleInflight singleflight.Group // linkedPath parameters whenever available. func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit profile *Profile, - newProfile bool, err error, ) { // Select correct key for single in flight. @@ -67,12 +66,10 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit if errors.Is(err, database.ErrNotFound) { switch id { case UnidentifiedProfileID: - profile = New(SourceLocal, UnidentifiedProfileID) - newProfile = true + profile = New(SourceLocal, UnidentifiedProfileID, linkedPath) err = nil case SystemProfileID: - profile = New(SourceLocal, SystemProfileID) - newProfile = true + profile = New(SourceLocal, SystemProfileID, linkedPath) err = nil } } @@ -90,7 +87,7 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit } } // Get from database. - profile, newProfile, err = findProfile(linkedPath) + profile, err = findProfile(linkedPath) default: return nil, errors.New("cannot fetch profile without ID or path") @@ -126,13 +123,13 @@ func GetProfile(source profileSource, id, linkedPath string) ( //nolint:gocognit return profile, nil }) if err != nil { - return nil, false, err + return nil, err } if p == nil { - return nil, false, errors.New("profile getter returned nil") + return nil, errors.New("profile getter returned nil") } - return p.(*Profile), newProfile, nil + return p.(*Profile), nil } // getProfile fetches the profile for the given scoped ID. @@ -149,7 +146,7 @@ func getProfile(scopedID string) (profile *Profile, err error) { // findProfile searches for a profile with the given linked path. If it cannot // find one, it will create a new profile for the given linked path. -func findProfile(linkedPath string) (profile *Profile, new bool, err error) { +func findProfile(linkedPath string) (profile *Profile, err error) { // Search the database for a matching profile. it, err := profileDB.Query( query.New(makeProfileKey(SourceLocal, "")).Where( @@ -157,7 +154,7 @@ func findProfile(linkedPath string) (profile *Profile, new bool, err error) { ), ) if err != nil { - return nil, false, err + return nil, err } // Only wait for the first result, or until the query ends. @@ -168,12 +165,11 @@ func findProfile(linkedPath string) (profile *Profile, new bool, err error) { // Prep and return an existing profile. if r != nil { profile, err = prepProfile(r) - return profile, false, err + return profile, err } // If there was no profile in the database, create a new one, and return it. - profile = New(SourceLocal, "") - profile.LinkedPath = linkedPath + profile = New(SourceLocal, "", linkedPath) // Check if the profile should be marked as internal. // This is the case whenever the binary resides within the data root dir. @@ -181,7 +177,7 @@ func findProfile(linkedPath string) (profile *Profile, new bool, err error) { profile.Internal = true } - return profile, true, nil + return profile, nil } func prepProfile(r record.Record) (*Profile, error) { diff --git a/profile/module_test.go b/profile/module_test.go index 55bd1876..06bb7f8d 100644 --- a/profile/module_test.go +++ b/profile/module_test.go @@ -1,11 +1,7 @@ package profile -import ( - "testing" - - "github.com/safing/portmaster/core/pmtesting" -) - +/* func TestMain(m *testing.M) { pmtesting.TestMain(m, module) } +*/ diff --git a/profile/profile-layered.go b/profile/profile-layered.go index 38471273..4f5d29f0 100644 --- a/profile/profile-layered.go +++ b/profile/profile-layered.go @@ -216,7 +216,7 @@ 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) if err != nil { log.Errorf("profiles: failed to update profile %s", layer.ScopedID()) } else { diff --git a/profile/profile.go b/profile/profile.go index 8bc6bf5a..665d6ff8 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -1,13 +1,17 @@ package profile import ( + "context" "errors" "fmt" + "path/filepath" "strings" "sync" "sync/atomic" "time" + "github.com/safing/portbase/utils/osdetail" + "github.com/tevino/abool" "github.com/safing/portbase/config" @@ -58,9 +62,9 @@ type Profile struct { //nolint:maligned // not worth the effort sync.RWMutex // ID is a unique identifier for the profile. - ID string + ID string // constant // Source describes the source of the profile. - Source profileSource + Source profileSource // constant // Name is a human readable name of the profile. It // defaults to the basename of the application. Name string @@ -78,7 +82,7 @@ type Profile struct { //nolint:maligned // not worth the effort IconType iconType // LinkedPath is a filesystem path to the executable this // profile was created for. - LinkedPath string + LinkedPath string // constant // LinkedProfiles is a list of other profiles LinkedProfiles []string // SecurityLevel is the mininum security level to apply to @@ -191,10 +195,11 @@ func (profile *Profile) parseConfig() error { } // New returns a new Profile. -func New(source profileSource, id string) *Profile { +func New(source profileSource, id string, linkedPath string) *Profile { profile := &Profile{ ID: id, Source: source, + LinkedPath: linkedPath, Created: time.Now().Unix(), Config: make(map[string]interface{}), internalSave: true, @@ -377,3 +382,106 @@ func EnsureProfile(r record.Record) (*Profile, error) { } return new, 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 (p *Profile) UpdateMetadata(processName string) (changed bool) { + // Check if this is a local profile, else warn and return. + if p.Source != SourceLocal { + log.Warningf("tried to update metadata for non-local profile %s", p.ScopedID()) + return false + } + + p.Lock() + defer p.Unlock() + + // Check if this is a special profile. + if p.LinkedPath == "" { + // This is a special profile, just assign the processName, if needed, and + // return. + if p.Name != processName { + p.Name = processName + return true + } + return false + } + + var needsUpdateFromSystem bool + + // Check profile name. + _, filename := filepath.Split(p.LinkedPath) + + // Update profile name if it is empty or equals the filename, which is the + // case for older profiles. + if p.Name == "" || p.Name == filename { + // Generate a default profile name if does not exist. + p.Name = osdetail.GenerateBinaryNameFromPath(p.LinkedPath) + if p.Name == filename { + // TODO: Theoretically, the generated name could be identical to the + // filename. + // As a quick fix, append a space to the name. + p.Name += " " + } + changed = true + needsUpdateFromSystem = true + } + + // If needed, get more/better data from the operating system. + if needsUpdateFromSystem { + module.StartWorker("get profile metadata", p.updateMetadataFromSystem) + } + + return changed +} + +// updateMetadataFromSystem updates the profile metadata with data from the +// operating system and saves it afterwards. +func (p *Profile) updateMetadataFromSystem(ctx context.Context) error { + // This function is only valid for local profiles. + if p.Source != SourceLocal || p.LinkedPath == "" { + return fmt.Errorf("tried to update metadata for non-local / non-linked profile %s", p.ScopedID()) + } + + // Save the profile when finished, if needed. + save := false + defer func() { + if save { + err := p.Save() + if err != nil { + log.Warningf("profile: failed to save %s after metadata update: %s", p.ScopedID(), err) + } + } + }() + + // Get binary name from linked path. + newName, err := osdetail.GetBinaryNameFromSystem(p.LinkedPath) + if err != nil { + log.Warningf("profile: error while getting binary name for %s: %s", p.LinkedPath, err) + return nil + } + + // Get filename of linked path for comparison. + _, filename := filepath.Split(p.LinkedPath) + + // 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 += " " + } + + // Lock profile for applying metadata. + p.Lock() + defer p.Unlock() + + // Apply new name if it changed. + if p.Name != newName { + p.Name = newName + save = true + } + + return nil +} From 8b60a6bb63be9b6183adaab82905817921cdff82 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 17 Nov 2020 10:13:33 +0100 Subject: [PATCH 2/2] Implement review suggestions --- profile/module_test.go | 7 ------- profile/profile.go | 3 +-- 2 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 profile/module_test.go diff --git a/profile/module_test.go b/profile/module_test.go deleted file mode 100644 index 06bb7f8d..00000000 --- a/profile/module_test.go +++ /dev/null @@ -1,7 +0,0 @@ -package profile - -/* -func TestMain(m *testing.M) { - pmtesting.TestMain(m, module) -} -*/ diff --git a/profile/profile.go b/profile/profile.go index 665d6ff8..b1798b07 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -10,14 +10,13 @@ import ( "sync/atomic" "time" - "github.com/safing/portbase/utils/osdetail" - "github.com/tevino/abool" "github.com/safing/portbase/config" "github.com/safing/portbase/database/record" "github.com/safing/portbase/log" "github.com/safing/portbase/utils" + "github.com/safing/portbase/utils/osdetail" "github.com/safing/portmaster/intel/filterlists" "github.com/safing/portmaster/profile/endpoints" )