github/portmaster
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #938 from safing/fix/verdict-and-profile-issues

Browse Source 
Fix verdict and profile issues
This commit is contained in:
Daniel Hovie
2022-10-13 15:38:22 +02:00
committed by GitHub
parent 7cff7df7b3 f5f9c52eb9
commit 32ec4677d3
12 changed files with 154 additions and 160 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
core/api.go
View File

@@ -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.",

36
firewall/interception.go
View File

@@ -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)

37
firewall/master.go
View File

@@ -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.

2
nameserver/nameserver.go
View File

@@ -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.

2
network/connection.go
View File

@@ -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())

27
process/profile.go
View File

@@ -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
}

6
profile/config-update.go
View File

@@ -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?

14
profile/config.go
View File

@@ -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

32
profile/get.go
View File

@@ -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)
}

8
profile/migrations.go
View File

@@ -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

6
profile/profile.go
View File

@@ -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)

130
profile/special.go
View File

@@ -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: