diff --git a/core/api.go b/core/api.go index 3ef37924..ffa02a28 100644 --- a/core/api.go +++ b/core/api.go @@ -25,9 +25,10 @@ import ( func registerAPIEndpoints() error { if err := api.RegisterEndpoint(api.Endpoint{ - Path: "core/shutdown", - Write: api.PermitSelf, - BelongsTo: module, + Path: "core/shutdown", + Write: api.PermitSelf, + // Do NOT register as belonging to the module, so that the API is available + // when something fails during starting of this module or a dependency. ActionFunc: shutdown, Name: "Shut Down Portmaster", Description: "Shut down the Portmaster Core Service and all UI components.", @@ -36,9 +37,10 @@ func registerAPIEndpoints() error { } if err := api.RegisterEndpoint(api.Endpoint{ - Path: "core/restart", - Write: api.PermitAdmin, - BelongsTo: module, + Path: "core/restart", + Write: api.PermitAdmin, + // Do NOT register as belonging to the module, so that the API is available + // when something fails during starting of this module or a dependency. ActionFunc: restart, Name: "Restart Portmaster", Description: "Restart the Portmaster Core Service.", diff --git a/firewall/interception.go b/firewall/interception.go index 85ffa01e..bc4395a7 100644 --- a/firewall/interception.go +++ b/firewall/interception.go @@ -112,7 +112,7 @@ func interceptionPrep() error { func resetAllConnectionVerdicts() { // Resetting will force all the connection to be evaluated by the firewall again // this will set new verdicts if configuration was update or spn has been disabled or enabled. - log.Info("interception: marking all connections for re-evaluation") + log.Info("interception: re-evaluating all connections") // Create tracing context. ctx, tracer := log.AddTracer(context.Background()) @@ -137,7 +137,7 @@ func resetAllConnectionVerdicts() { previousVerdict := conn.Verdict.Firewall // Apply privacy filter and check tunneling. - filterConnection(ctx, conn, nil) + FilterConnection(ctx, conn, nil, true, true) // Stop existing SPN tunnel if not needed anymore. if conn.Verdict.Active != network.VerdictRerouteToTunnel && conn.TunnelContext != nil { @@ -157,7 +157,7 @@ func resetAllConnectionVerdicts() { } }() } - tracer.Infof("profile: changed verdict on %d connections", changedVerdicts) + tracer.Infof("filter: changed verdict on %d connections", changedVerdicts) tracer.Submit() err := interception.ResetVerdictOfAllConnections() @@ -437,14 +437,17 @@ func fastTrackedPermit(pkt packet.Packet) (handled bool) { } func initialHandler(conn *network.Connection, pkt packet.Packet) { - log.Tracer(pkt.Ctx()).Trace("filter: handing over to connection-based handler") + filterConnection := true + log.Tracer(pkt.Ctx()).Trace("filter: handing over to connection-based handler") // Check for special (internal) connection cases. switch { case !conn.Inbound && localPortIsPreAuthenticated(conn.Entity.Protocol, conn.LocalPort): // Approve connection. conn.Accept("connection by Portmaster", noReasonOptionKey) conn.Internal = true + filterConnection = false + log.Tracer(pkt.Ctx()).Infof("filter: granting own pre-authenticated connection %s", conn) // Redirect outbound DNS packets if enabled, case dnsQueryInterception() && @@ -461,9 +464,9 @@ func initialHandler(conn *network.Connection, pkt packet.Packet) { conn.Entity.IPScope == netutils.LocalMulticast): // Reroute rogue dns queries back to Portmaster. - conn.SetVerdictDirectly(network.VerdictRerouteToNameserver) - conn.Reason.Msg = "redirecting rogue dns query" + conn.SetVerdict(network.VerdictRerouteToNameserver, "redirecting rogue dns query", "", nil) conn.Internal = true + log.Tracer(pkt.Ctx()).Infof("filter: redirecting dns query %s to Portmaster", conn) // End directly, as no other processing is necessary. conn.StopFirewallHandler() finalizeVerdict(conn) @@ -472,7 +475,7 @@ func initialHandler(conn *network.Connection, pkt packet.Packet) { } // Apply privacy filter and check tunneling. - filterConnection(pkt.Ctx(), conn, pkt) + FilterConnection(pkt.Ctx(), conn, pkt, filterConnection, true) // Decide how to continue handling connection. switch { @@ -486,12 +489,15 @@ func initialHandler(conn *network.Connection, pkt packet.Packet) { } } -func filterConnection(ctx context.Context, conn *network.Connection, pkt packet.Packet) { - if filterEnabled() { - log.Tracer(ctx).Trace("filter: starting decision process") - DecideOnConnection(ctx, conn, pkt) - } else { - conn.Accept("privacy filter disabled", noReasonOptionKey) +// FilterConnection runs all the filtering (and tunneling) procedures. +func FilterConnection(ctx context.Context, conn *network.Connection, pkt packet.Packet, checkFilter, checkTunnel bool) { + if checkFilter { + if filterEnabled() { + log.Tracer(ctx).Trace("filter: starting decision process") + decideOnConnection(ctx, conn, pkt) + } else { + conn.Accept("privacy filter disabled", noReasonOptionKey) + } } // TODO: Enable inspection framework again. @@ -511,7 +517,9 @@ func filterConnection(ctx context.Context, conn *network.Connection, pkt packet. } // Check if connection should be tunneled. - checkTunneling(ctx, conn) + if checkTunnel { + checkTunneling(ctx, conn) + } // Handle verdict records and transitions. finalizeVerdict(conn) diff --git a/firewall/master.go b/firewall/master.go index bd7a9c7d..349f7577 100644 --- a/firewall/master.go +++ b/firewall/master.go @@ -58,9 +58,9 @@ var defaultDeciders = []deciderFn{ checkAutoPermitRelated, } -// DecideOnConnection makes a decision about a connection. +// decideOnConnection makes a decision about a connection. // When called, the connection and profile is already locked. -func DecideOnConnection(ctx context.Context, conn *network.Connection, pkt packet.Packet) { +func decideOnConnection(ctx context.Context, conn *network.Connection, pkt packet.Packet) { // Check if we have a process and profile. layeredProfile := conn.Process().Profile() if layeredProfile == nil { @@ -141,15 +141,34 @@ func runDeciders(ctx context.Context, selectedDeciders []deciderFn, conn *networ // checkPortmasterConnection allows all connection that originate from // portmaster itself. func checkPortmasterConnection(ctx context.Context, conn *network.Connection, _ *profile.LayeredProfile, _ packet.Packet) bool { - // Grant own outgoing connections. - if conn.Process().Pid == ownPID && !conn.Inbound { - log.Tracer(ctx).Infof("filter: granting own connection %s", conn) - conn.Accept("connection by Portmaster", noReasonOptionKey) - conn.Internal = true - return true + // Grant own outgoing or local connections. + + // Blocking our own connections can lead to a very literal deadlock. + // This can currently happen, as fast-tracked connections are also + // reset in the OS integration and might show up in the connection + // handling if a packet in the other direction hits the firewall first. + + // Ignore other processes. + if conn.Process().Pid != ownPID { + return false } - return false + // Ignore inbound connection if non-local. + if conn.Inbound { + myIP, err := netenv.IsMyIP(conn.Entity.IP) + if err != nil { + log.Tracer(ctx).Debugf("filter: failed to check if %s is own IP for granting own connection: %s", conn.Entity.IP, err) + return false + } + if !myIP { + return false + } + } + + log.Tracer(ctx).Infof("filter: granting own connection %s", conn) + conn.Accept("connection by Portmaster", noReasonOptionKey) + conn.Internal = true + return true } // checkSelfCommunication checks if the process is communicating with itself. diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 0d000a1e..a13e98cd 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -222,7 +222,7 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) }() // Check request with the privacy filter before resolving. - firewall.DecideOnConnection(ctx, conn, nil) + firewall.FilterConnection(ctx, conn, nil, true, false) // Check if there is a responder from the firewall. // In special cases, the firewall might want to respond the query itself. diff --git a/network/connection.go b/network/connection.go index d87964f9..3d5ba694 100644 --- a/network/connection.go +++ b/network/connection.go @@ -333,7 +333,7 @@ func NewConnectionFromFirstPacket(pkt packet.Packet) *Connection { proc, inbound, err := process.GetProcessByConnection(pkt.Ctx(), pkt.Info()) if err != nil { log.Tracer(pkt.Ctx()).Debugf("network: failed to find process of packet %s: %s", pkt, err) - if inbound { + if inbound && !netutils.ClassifyIP(pkt.Info().Dst).IsLocalhost() { proc = process.GetUnsolicitedProcess(pkt.Ctx()) } else { proc = process.GetUnidentifiedProcess(pkt.Ctx()) diff --git a/process/profile.go b/process/profile.go index 9c49e832..65680639 100644 --- a/process/profile.go +++ b/process/profile.go @@ -27,18 +27,10 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { // If not, continue with loading the profile. log.Tracer(ctx).Trace("process: loading profile") - // Check if there is a special profile for this process. - localProfile, err := p.loadSpecialProfile(ctx) + // Get special or regular profile. + localProfile, err := profile.GetLocalProfile(p.getSpecialProfileID(), p.MatchingData(), p.CreateProfileCallback) if err != nil { - return false, fmt.Errorf("failed to load special profile: %w", err) - } - - // Otherwise, find a regular profile for the process. - if localProfile == nil { - localProfile, err = profile.GetLocalProfile("", p.MatchingData(), p.CreateProfileCallback) - if err != nil { - return false, fmt.Errorf("failed to find profile: %w", err) - } + return false, fmt.Errorf("failed to find profile: %w", err) } // Assign profile to process. @@ -48,10 +40,9 @@ func (p *Process) GetProfile(ctx context.Context) (changed bool, err error) { return true, nil } -// loadSpecialProfile attempts to load a special profile. -func (p *Process) loadSpecialProfile(_ context.Context) (*profile.Profile, error) { +// getSpecialProfileID returns the special profile ID for the process, if any. +func (p *Process) getSpecialProfileID() (specialProfileID string) { // Check if we need a special profile. - var specialProfileID string switch p.Pid { case UnidentifiedProcessID: specialProfileID = profile.UnidentifiedProfileID @@ -103,11 +94,5 @@ func (p *Process) loadSpecialProfile(_ context.Context) (*profile.Profile, error } } - // Check if a special profile should be applied. - if specialProfileID == "" { - return nil, nil - } - - // Return special profile. - return profile.GetSpecialProfile(specialProfileID, p.Path) + return specialProfileID } diff --git a/profile/config-update.go b/profile/config-update.go index 6f249548..1e73d4fe 100644 --- a/profile/config-update.go +++ b/profile/config-update.go @@ -44,11 +44,11 @@ func updateGlobalConfigProfile(ctx context.Context, task *modules.Task) error { action := cfgOptionDefaultAction() switch action { - case "permit": + case DefaultActionPermitValue: cfgDefaultAction = DefaultActionPermit - case "ask": + case DefaultActionAskValue: cfgDefaultAction = DefaultActionAsk - case "block": + case DefaultActionBlockValue: cfgDefaultAction = DefaultActionBlock default: // TODO: module error? diff --git a/profile/config.go b/profile/config.go index 4da74512..9ca174ed 100644 --- a/profile/config.go +++ b/profile/config.go @@ -23,6 +23,10 @@ var ( cfgOptionDefaultAction config.StringOption cfgOptionDefaultActionOrder = 1 + DefaultActionPermitValue = "permit" + DefaultActionBlockValue = "block" + DefaultActionAskValue = "ask" + // Setting "Prompt Desktop Notifications" at order 2. // Setting "Prompt Timeout" at order 3. @@ -182,7 +186,7 @@ func registerConfiguration() error { //nolint:maintidx Key: CfgOptionDefaultActionKey, Description: `The default network action is applied when nothing else allows or blocks an outgoing connection. Incoming connections are always blocked by default.`, OptType: config.OptTypeString, - DefaultValue: "permit", + DefaultValue: DefaultActionPermitValue, Annotations: config.Annotations{ config.DisplayHintAnnotation: config.DisplayHintOneOf, config.DisplayOrderAnnotation: cfgOptionDefaultActionOrder, @@ -191,17 +195,17 @@ func registerConfiguration() error { //nolint:maintidx PossibleValues: []config.PossibleValue{ { Name: "Allow", - Value: "permit", + Value: DefaultActionPermitValue, Description: "Allow all connections", }, { Name: "Block", - Value: "block", + Value: DefaultActionBlockValue, Description: "Block all connections", }, { Name: "Prompt", - Value: "ask", + Value: DefaultActionAskValue, Description: "Prompt for decisions", }, }, @@ -209,7 +213,7 @@ func registerConfiguration() error { //nolint:maintidx if err != nil { return err } - cfgOptionDefaultAction = config.Concurrent.GetAsString(CfgOptionDefaultActionKey, "permit") + cfgOptionDefaultAction = config.Concurrent.GetAsString(CfgOptionDefaultActionKey, DefaultActionPermitValue) cfgStringOptions[CfgOptionDefaultActionKey] = cfgOptionDefaultAction // Disable Auto Permit diff --git a/profile/get.go b/profile/get.go index 9d6d1b61..b9cc360f 100644 --- a/profile/get.go +++ b/profile/get.go @@ -7,6 +7,7 @@ import ( "strings" "sync" + "github.com/safing/portbase/database" "github.com/safing/portbase/database/query" "github.com/safing/portbase/database/record" "github.com/safing/portbase/log" @@ -50,6 +51,7 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P // In some cases, we might need to get a profile directly, without matching data. // This could lead to inconsistent data - use with caution. + // Example: Saving prompt results to profile should always be to the same ID! if md == nil { if id == "" { return nil, errors.New("cannot get local profiles without ID and matching data") @@ -61,6 +63,26 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P } } + // Check if we are requesting a special profile. + var created, special bool + if id != "" && isSpecialProfileID(id) { + special = true + + // Get special profile from DB. + if profile == nil { + profile, err = getProfile(makeScopedID(SourceLocal, id)) + if err != nil && !errors.Is(err, database.ErrNotFound) { + log.Warningf("profile: failed to get special profile %s: %s", id, err) + } + } + + // Create profile if not found or if it needs a reset. + if profile == nil || specialProfileNeedsReset(profile) { + profile = createSpecialProfile(id, md.Path()) + created = true + } + } + // If we don't have a profile yet, find profile based on matching data. if profile == nil { profile, err = findProfile(SourceLocal, md) @@ -70,7 +92,6 @@ 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 @@ -105,7 +126,12 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P // Initialize and update profile. // Update metadata. - changed := profile.updateMetadata(md.Path()) + var changed bool + if special { + changed = updateSpecialProfileMetadata(profile, md.Path()) + } else { + changed = profile.updateMetadata(md.Path()) + } // Save if created or changed. if created || changed { @@ -117,7 +143,7 @@ func GetLocalProfile(id string, md MatchingData, createProfileCallback func() *P } // Trigger further metadata fetching from system if profile was created. - if created && profile.UsePresentationPath { + if created && profile.UsePresentationPath && !special { module.StartWorker("get profile metadata", profile.updateMetadataFromSystem) } diff --git a/profile/migrations.go b/profile/migrations.go index aee673bd..5db78958 100644 --- a/profile/migrations.go +++ b/profile/migrations.go @@ -2,7 +2,6 @@ package profile import ( "context" - "fmt" "github.com/hashicorp/go-version" @@ -62,7 +61,8 @@ func migrateLinkedPath(ctx context.Context, _, to *version.Version, db *database // Get iterator over all profiles. it, err := db.Query(query.New(profilesDBPath)) if err != nil { - return fmt.Errorf("failed to query profiles: %w", err) + log.Tracer(ctx).Errorf("profile: failed to migrate from linked path: failed to start query: %s", err) + return nil } // Migrate all profiles. @@ -91,8 +91,8 @@ func migrateLinkedPath(ctx context.Context, _, to *version.Version, db *database } // Check if there was an error while iterating. - if it.Err() != nil { - return fmt.Errorf("profiles: failed to iterate over profiles for migration: %w", err) + if err := it.Err(); err != nil { + log.Tracer(ctx).Errorf("profile: failed to migrate from linked path: failed to iterate over profiles for migration: %s", err) } return nil diff --git a/profile/profile.go b/profile/profile.go index fb56d90c..d6bc8eaa 100644 --- a/profile/profile.go +++ b/profile/profile.go @@ -175,11 +175,11 @@ func (profile *Profile) parseConfig() error { profile.defaultAction = DefaultActionNotSet if ok { switch action { - case "permit": + case DefaultActionPermitValue: profile.defaultAction = DefaultActionPermit - case "ask": + case DefaultActionAskValue: profile.defaultAction = DefaultActionAsk - case "block": + case DefaultActionBlockValue: profile.defaultAction = DefaultActionBlock default: lastErr = fmt.Errorf(`default action "%s" invalid`, action) diff --git a/profile/special.go b/profile/special.go index 79c2a7b5..106d294f 100644 --- a/profile/special.go +++ b/profile/special.go @@ -1,10 +1,8 @@ package profile import ( - "errors" "time" - "github.com/safing/portbase/database" "github.com/safing/portbase/log" "github.com/safing/portmaster/status" ) @@ -13,7 +11,7 @@ const ( // UnidentifiedProfileID is the profile ID used for unidentified processes. UnidentifiedProfileID = "_unidentified" // UnidentifiedProfileName is the name used for unidentified processes. - UnidentifiedProfileName = "Unidentified App" + UnidentifiedProfileName = "Other Connections" // UnidentifiedProfileDescription is the description used for unidentified processes. UnidentifiedProfileDescription = `Connections that could not be attributed to a specific app. @@ -77,91 +75,19 @@ If you think you might have messed up the settings of the System DNS Client, jus PortmasterNotifierProfileDescription = `This is the Portmaster UI Tray Notifier.` ) -// GetSpecialProfile fetches a special profile. This function ensures that the loaded profile -// is shared among all callers. Always provide all available data points. -func GetSpecialProfile(id string, path string) ( //nolint:gocognit - profile *Profile, - err error, -) { - // Check if we have an ID. - if id == "" { - return nil, errors.New("cannot get special profile without ID") - } - scopedID := makeScopedID(SourceLocal, id) - - // Globally lock getting a profile. - // This does not happen too often, and it ensures we really have integrity - // and no race conditions. - getProfileLock.Lock() - defer getProfileLock.Unlock() - - // Check if there already is an active profile. - var previousVersion *Profile - profile = getActiveProfile(scopedID) - if profile != nil { - // Mark active and return if not outdated. - if profile.outdated.IsNotSet() { - profile.MarkStillActive() - return profile, nil - } - - // If outdated, get from database. - previousVersion = profile - } - - // Get special profile from DB and check if it needs a reset. - var created bool - profile, err = getProfile(scopedID) - 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 +func isSpecialProfileID(id string) bool { + switch id { + case UnidentifiedProfileID, + UnsolicitedProfileID, + SystemProfileID, + SystemResolverProfileID, + PortmasterProfileID, + PortmasterAppProfileID, + PortmasterNotifierProfileID: + return true default: - // Create new profile if it does not exist (or failed to load). - profile = createSpecialProfile(id, path) - created = true + return false } - // 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. - // The internal references will be updated when the layered profile checks for updates. - if previousVersion != nil && previousVersion.layeredProfile != nil { - profile.layeredProfile = previousVersion.layeredProfile - } - - // Profiles must have a layered profile, create a new one if it - // does not yet exist. - if profile.layeredProfile == nil { - profile.layeredProfile = NewLayeredProfile(profile) - } - - // Add the profile to the currently active profiles. - addActiveProfile(profile) - - return profile, nil } func updateSpecialProfileMetadata(profile *Profile, binaryPath string) (changed bool) { @@ -248,7 +174,7 @@ func createSpecialProfile(profileID string, path string) *Profile { // Resolved domain from the system resolver are checked again when // attributed to a connection of a regular process. Otherwise, users // would see two connection prompts for the same domain. - CfgOptionDefaultActionKey: "permit", + CfgOptionDefaultActionKey: DefaultActionPermitValue, // Explicitly allow incoming connections. CfgOptionBlockInboundKey: status.SecurityLevelOff, // Explicitly allow localhost and answers to multicast protocols that @@ -276,7 +202,29 @@ func createSpecialProfile(profileID string, path string) *Profile { ID: PortmasterProfileID, Source: SourceLocal, PresentationPath: path, - Internal: true, + Config: map[string]interface{}{ + // In case anything slips through the internal self-allow, be sure to + // allow everything explicitly. + // Blocking connections here can lead to a very literal deadlock. + // This can currently happen, as fast-tracked connections are also + // reset in the OS integration and might show up in the connection + // handling if a packet in the other direction hits the firewall first. + CfgOptionDefaultActionKey: DefaultActionPermitValue, + CfgOptionBlockScopeInternetKey: status.SecurityLevelOff, + CfgOptionBlockScopeLANKey: status.SecurityLevelOff, + CfgOptionBlockScopeLocalKey: status.SecurityLevelOff, + CfgOptionBlockP2PKey: status.SecurityLevelOff, + CfgOptionBlockInboundKey: status.SecurityLevelOff, + CfgOptionEndpointsKey: []string{ + "+ *", + }, + CfgOptionServiceEndpointsKey: []string{ + "+ Localhost", + "+ LAN", + "- *", + }, + }, + Internal: true, }) case PortmasterAppProfileID: @@ -285,7 +233,7 @@ func createSpecialProfile(profileID string, path string) *Profile { Source: SourceLocal, PresentationPath: path, Config: map[string]interface{}{ - CfgOptionDefaultActionKey: "block", + CfgOptionDefaultActionKey: DefaultActionBlockValue, CfgOptionEndpointsKey: []string{ "+ Localhost", "+ .safing.io", @@ -300,7 +248,7 @@ func createSpecialProfile(profileID string, path string) *Profile { Source: SourceLocal, PresentationPath: path, Config: map[string]interface{}{ - CfgOptionDefaultActionKey: "block", + CfgOptionDefaultActionKey: DefaultActionBlockValue, CfgOptionEndpointsKey: []string{ "+ Localhost", }, @@ -334,6 +282,8 @@ func specialProfileNeedsReset(profile *Profile) bool { switch profile.ID { case SystemResolverProfileID: return canBeUpgraded(profile, "21.10.2022") + case PortmasterProfileID: + return canBeUpgraded(profile, "21.10.2022") case PortmasterAppProfileID: return canBeUpgraded(profile, "8.9.2021") default: