Merge pull request #1264 from safing/fix/1.2+1.3

Fix v1.2 and v1.3 bugs

broadcasts/data.go

@@ -9,6 +9,7 @@ import (
 	"github.com/safing/portmaster/netenv"
 	"github.com/safing/portmaster/updates"
 	"github.com/safing/spn/access"
+	"github.com/safing/spn/access/account"
 	"github.com/safing/spn/captain"
 )
 
@@ -67,11 +68,16 @@ func collectData() interface{} {
 			Error: err,
 		}
 	} else {
-		data["Account"] = &Account{
+		account := &Account{
 			UserRecord: userRecord,
+			Active:     userRecord.MayUse(""),
 			UpToDate:   userRecord.Meta().Modified > time.Now().Add(-7*24*time.Hour).Unix(),
-			MayUseSPN:  userRecord.MayUseSPN(),
 		}
+		// Only add feature IDs when account is active.
+		if account.Active {
+			account.FeatureIDs = userRecord.CurrentPlan.FeatureIDs
+		}
+		data["Account"] = account
 	}
 
 	// Time running.
@@ -101,8 +107,9 @@ type Location struct {
 // Account holds SPN account matching data.
 type Account struct {
 	*access.UserRecord
-	UpToDate  bool
-	MayUseSPN bool
+	Active     bool
+	UpToDate   bool
+	FeatureIDs []account.FeatureID
 }
 
 // DataError represents an error getting some matching data.

firewall/interception/ebpf/bandwidth/interface.go

@@ -28,6 +28,10 @@ func BandwidthStatsWorker(ctx context.Context, collectInterval time.Duration, ba
 	// Allow the current process to lock memory for eBPF resources.
 	err := rlimit.RemoveMemlock()
 	if err != nil {
+		if ebpfLoadingFailed.Add(1) >= 5 {
+			log.Warningf("ebpf: failed to remove memlock 5 times, giving up with error %s", err)
+			return nil
+		}
 		return fmt.Errorf("ebpf: failed to remove memlock: %w", err)
 	}
 

firewall/interception/ebpf/connection_listener/bpf_bpfeb.go

@@ -1,6 +1,5 @@
 // Code generated by bpf2go; DO NOT EDIT.
 //go:build arm64be || armbe || mips || mips64 || mips64p32 || ppc64 || s390 || s390x || sparc || sparc64
-// +build arm64be armbe mips mips64 mips64p32 ppc64 s390 s390x sparc sparc64
 
 package ebpf
 

firewall/interception/ebpf/connection_listener/bpf_bpfel.go

@@ -1,6 +1,5 @@
 // Code generated by bpf2go; DO NOT EDIT.
-//go:build 386 || amd64 || amd64p32 || arm || arm64 || mips64le || mips64p32le || mipsle || ppc64le || riscv64
-// +build 386 amd64 amd64p32 arm arm64 mips64le mips64p32le mipsle ppc64le riscv64
+//go:build 386 || amd64 || amd64p32 || arm || arm64 || loong64 || mips64le || mips64p32le || mipsle || ppc64le || riscv64
 
 package ebpf
 

firewall/interception/ebpf/connection_listener/worker.go

@@ -26,6 +26,10 @@ var ebpfLoadingFailed atomic.Uint32
 func ConnectionListenerWorker(ctx context.Context, packets chan packet.Packet) error {
 	// Allow the current process to lock memory for eBPF resources.
 	if err := rlimit.RemoveMemlock(); err != nil {
+		if ebpfLoadingFailed.Add(1) >= 5 {
+			log.Warningf("ebpf: failed to remove memlock 5 times, giving up with error %s", err)
+			return nil
+		}
 		return fmt.Errorf("ebpf: failed to remove ebpf memlock: %w", err)
 	}
 

firewall/interception/ebpf/programs/monitor.c

@@ -46,8 +46,8 @@ int BPF_PROG(tcp_connect, struct sock *sk) {
 		return 0;
 	}
 
-  // Read PID
-	tcp_info->pid = __builtin_bswap32((u32)bpf_get_current_pid_tgid());
+  // Read PID (Careful: This is the Thread Group ID in kernel speak!)
+	tcp_info->pid = __builtin_bswap32((u32)(bpf_get_current_pid_tgid() >> 32));
 
 	// Set protocol
 	tcp_info->protocol = TCP;

netquery/database.go

@@ -7,7 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"path"
+	"path/filepath"
 	"sort"
 	"strings"
 	"sync"
@@ -124,7 +124,10 @@ func New(dbPath string) (*Database, error) {
 		return nil, fmt.Errorf("failed to ensure database directory exists: %w", err)
 	}
 
-	historyPath := "file://" + path.Join(historyParentDir.Path, "history.db")
+	// Get file location of history database.
+	historyFile := filepath.Join(historyParentDir.Path, "history.db")
+	// Convert to SQLite URI path.
+	historyURI := "file:///" + strings.TrimPrefix(filepath.ToSlash(historyFile), "/")
 
 	constructor := func(ctx context.Context) (*sqlite.Conn, error) {
 		c, err := sqlite.OpenConn(
@@ -137,7 +140,7 @@ func New(dbPath string) (*Database, error) {
 			return nil, fmt.Errorf("failed to open read-only sqlite connection at %s: %w", dbPath, err)
 		}
 
-		if err := sqlitex.ExecuteTransient(c, "ATTACH DATABASE '"+historyPath+"?mode=ro' AS history", nil); err != nil {
+		if err := sqlitex.ExecuteTransient(c, "ATTACH DATABASE '"+historyURI+"?mode=ro' AS history", nil); err != nil {
 			return nil, fmt.Errorf("failed to attach history database: %w", err)
 		}
 
@@ -180,7 +183,7 @@ func New(dbPath string) (*Database, error) {
 		readConnPool: pool,
 		Schema:       schema,
 		writeConn:    writeConn,
-		historyPath:  historyPath,
+		historyPath:  historyURI,
 	}, nil
 }
 
@@ -207,7 +210,6 @@ func NewInMemory() (*Database, error) {
 // any data-migrations. Once the history module is implemented this should
 // become/use a full migration system -- use zombiezen.com/go/sqlite/sqlitemigration.
 func (db *Database) ApplyMigrations() error {
-	log.Errorf("applying migrations ...")
 	db.l.Lock()
 	defer db.l.Unlock()
 

netquery/module_api.go

@@ -64,6 +64,7 @@ func (m *module) prepare() error {
 		Internal: true,
 	})
 
+	// TODO: Open database in start() phase.
 	m.Store, err = NewInMemory()
 	if err != nil {
 		return fmt.Errorf("failed to create in-memory database: %w", err)

network/clean.go

@@ -69,13 +69,16 @@ func cleanConnections() (activePIDs map[int]struct{}) {
 					PID:      process.UndefinedProcessID,
 				}, now)
 
-				activePIDs[conn.process.Pid] = struct{}{}
-
+				// Step 2: mark as ended
 				if !exists {
-					// Step 2: mark end
 					conn.Ended = nowUnix
 					conn.Save()
 				}
+
+				// If the connection has an associated process, add its PID to the active PID list.
+				if conn.process != nil {
+					activePIDs[conn.process.Pid] = struct{}{}
+				}
 			case conn.Ended < deleteOlderThan:
 				// Step 3: delete
 				// DEBUG:

network/connection.go

@@ -2,6 +2,7 @@ package network
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"strings"
@@ -338,7 +339,7 @@ func NewConnectionFromDNSRequest(ctx context.Context, fqdn string, cnames []stri
 	if localProfile := proc.Profile().LocalProfile(); localProfile != nil {
 		dnsConn.Internal = localProfile.Internal
 
-		if err := dnsConn.updateFeatures(); err != nil {
+		if err := dnsConn.updateFeatures(); err != nil && !errors.Is(err, access.ErrNotLoggedIn) {
 			log.Tracer(ctx).Warningf("network: failed to check for enabled features: %s", err)
 		}
 	}
@@ -380,7 +381,7 @@ func NewConnectionFromExternalDNSRequest(ctx context.Context, fqdn string, cname
 	if localProfile := remoteHost.Profile().LocalProfile(); localProfile != nil {
 		dnsConn.Internal = localProfile.Internal
 
-		if err := dnsConn.updateFeatures(); err != nil {
+		if err := dnsConn.updateFeatures(); err != nil && !errors.Is(err, access.ErrNotLoggedIn) {
 			log.Tracer(ctx).Warningf("network: failed to check for enabled features: %s", err)
 		}
 	}
@@ -448,7 +449,7 @@ func (conn *Connection) GatherConnectionInfo(pkt packet.Packet) (err error) {
 			if localProfile := conn.process.Profile().LocalProfile(); localProfile != nil {
 				conn.Internal = localProfile.Internal
 
-				if err := conn.updateFeatures(); err != nil {
+				if err := conn.updateFeatures(); err != nil && !errors.Is(err, access.ErrNotLoggedIn) {
 					log.Tracer(pkt.Ctx()).Warningf("network: failed to check for enabled features: %s", err)
 				}
 			}
@@ -656,14 +657,21 @@ func (conn *Connection) Failed(reason, reasonOptionKey string) {
 func (conn *Connection) SetVerdict(newVerdict Verdict, reason, reasonOptionKey string, reasonCtx interface{}) (ok bool) {
 	conn.SetVerdictDirectly(newVerdict)
 
+	// Set reason and context.
 	conn.Reason.Msg = reason
 	conn.Reason.Context = reasonCtx
 
+	// Reset option key.
 	conn.Reason.OptionKey = ""
 	conn.Reason.Profile = ""
-	if reasonOptionKey != "" && conn.Process() != nil {
-		conn.Reason.OptionKey = reasonOptionKey
-		conn.Reason.Profile = conn.Process().Profile().GetProfileSource(conn.Reason.OptionKey)
+
+	// Set option key if data is available.
+	if reasonOptionKey != "" {
+		lp := conn.Process().Profile()
+		if lp != nil {
+			conn.Reason.OptionKey = reasonOptionKey
+			conn.Reason.Profile = lp.GetProfileSource(conn.Reason.OptionKey)
+		}
 	}
 
 	return true // TODO: remove

process/database.go

@@ -111,7 +111,8 @@ func CleanProcessStorage(activePIDs map[int]struct{}) {
 		// The PID of a process does not change.
 
 		// Check if this is a special process.
-		if p.Pid == UnidentifiedProcessID || p.Pid == SystemProcessID {
+		switch p.Pid {
+		case UnidentifiedProcessID, UnsolicitedProcessID, SystemProcessID:
 			p.profile.MarkStillActive()
 			continue
 		}

updates/assets/portmaster.service

@@ -0,0 +1,44 @@
+[Unit]
+Description=Portmaster by Safing
+Documentation=https://safing.io
+Documentation=https://docs.safing.io
+Before=nss-lookup.target network.target shutdown.target
+After=systemd-networkd.service
+Conflicts=shutdown.target
+Conflicts=firewalld.service
+Wants=nss-lookup.target
+
+[Service]
+Type=simple
+Restart=on-failure
+RestartSec=10
+LockPersonality=yes
+MemoryDenyWriteExecute=yes
+NoNewPrivileges=yes
+PrivateTmp=yes
+PIDFile=/opt/safing/portmaster/core-lock.pid
+Environment=LOGLEVEL=info
+Environment=PORTMASTER_ARGS=
+EnvironmentFile=-/etc/default/portmaster
+ProtectSystem=true
+#ReadWritePaths=/var/lib/portmaster
+#ReadWritePaths=/run/xtables.lock
+RestrictAddressFamilies=AF_UNIX AF_NETLINK AF_INET AF_INET6
+RestrictNamespaces=yes
+# In future version portmaster will require access to user home
+# directories to verify application permissions.
+ProtectHome=read-only
+ProtectKernelTunables=yes
+ProtectKernelLogs=yes
+ProtectControlGroups=yes
+PrivateDevices=yes
+AmbientCapabilities=cap_chown cap_kill cap_net_admin cap_net_bind_service cap_net_broadcast cap_net_raw cap_sys_module cap_sys_ptrace cap_dac_override cap_fowner cap_fsetid cap_sys_resource cap_bpf cap_perfmon
+CapabilityBoundingSet=cap_chown cap_kill cap_net_admin cap_net_bind_service cap_net_broadcast cap_net_raw cap_sys_module cap_sys_ptrace cap_dac_override cap_fowner cap_fsetid cap_sys_resource cap_bpf cap_perfmon
+# SystemCallArchitectures=native
+# SystemCallFilter=@system-service @module
+# SystemCallErrorNumber=EPERM
+ExecStart=/opt/safing/portmaster/portmaster-start --data /opt/safing/portmaster core -- $PORTMASTER_ARGS
+ExecStopPost=-/opt/safing/portmaster/portmaster-start recover-iptables
+
+[Install]
+WantedBy=multi-user.target

updates/helper/electron.go

@@ -51,7 +51,7 @@ func EnsureChromeSandboxPermissions(reg *updater.ResourceRegistry) error {
 
 		return fmt.Errorf("failed to chmod: %w", err)
 	}
-	log.Infof("updates: fixed SUID permission for chrome-sandbox")
+	log.Debugf("updates: fixed SUID permission for chrome-sandbox")
 
 	return nil
 }

updates/helper/updates.go

@@ -44,7 +44,7 @@ func MandatoryUpdates() (identifiers []string) {
 
 	// Stop here if we only want intel data.
 	if intelOnly.IsSet() {
-		return
+		return identifiers
 	}
 
 	// Binaries

updates/main.go

@@ -284,13 +284,13 @@ func checkForUpdates(ctx context.Context) (err error) {
 
 	if err = registry.UpdateIndexes(ctx); err != nil {
 		err = fmt.Errorf("failed to update indexes: %w", err)
-		return
+		return //nolint:nakedret // TODO: Would "return err" work with the defer?
 	}
 
 	err = registry.DownloadUpdates(ctx, !forcedUpdate)
 	if err != nil {
 		err = fmt.Errorf("failed to download updates: %w", err)
-		return
+		return //nolint:nakedret // TODO: Would "return err" work with the defer?
 	}
 
 	registry.SelectVersions()
@@ -299,7 +299,7 @@ func checkForUpdates(ctx context.Context) (err error) {
 	err = registry.UnpackResources()
 	if err != nil {
 		err = fmt.Errorf("failed to unpack updates: %w", err)
-		return
+		return //nolint:nakedret // TODO: Would "return err" work with the defer?
 	}
 
 	// Purge old resources

updates/os_integration_default.go

@@ -0,0 +1,8 @@
+//go:build !linux
+// +build !linux
+
+package updates
+
+func upgradeSystemIntegration() error {
+	return nil
+}

updates/os_integration_linux.go

@@ -0,0 +1,204 @@
+package updates
+
+import (
+	"bytes"
+	"crypto/sha256"
+	_ "embed"
+	"encoding/hex"
+	"errors"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"path/filepath"
+
+	"github.com/tevino/abool"
+	"golang.org/x/exp/slices"
+
+	"github.com/safing/portbase/dataroot"
+	"github.com/safing/portbase/log"
+	"github.com/safing/portbase/utils/renameio"
+)
+
+var (
+	portmasterCoreServiceFilePath     = "portmaster.service"
+	portmasterNotifierServiceFilePath = "portmaster_notifier.desktop"
+	backupExtension                   = ".backup"
+
+	//go:embed assets/portmaster.service
+	currentPortmasterCoreServiceFile []byte
+
+	checkedSystemIntegration = abool.New()
+
+	// ErrRequiresManualUpgrade is returned when a system integration file requires a manual upgrade.
+	ErrRequiresManualUpgrade = errors.New("requires a manual upgrade")
+)
+
+func upgradeSystemIntegration() {
+	// Check if we already checked the system integration.
+	if !checkedSystemIntegration.SetToIf(false, true) {
+		return
+	}
+
+	// Upgrade portmaster core systemd service.
+	err := upgradeSystemIntegrationFile(
+		"portmaster core systemd service",
+		filepath.Join(dataroot.Root().Path, portmasterCoreServiceFilePath),
+		0o0600,
+		currentPortmasterCoreServiceFile,
+		[]string{
+			"bc26dd37e6953af018ad3676ee77570070e075f2b9f5df6fa59d65651a481468", // Commit 19c76c7 on 2022-01-25
+			"cc0cb49324dfe11577e8c066dd95cc03d745b50b2153f32f74ca35234c3e8cb5", // Commit ef479e5 on 2022-01-24
+			"d08a3b5f3aee351f8e120e6e2e0a089964b94c9e9d0a9e5fa822e60880e315fd", // Commit b64735e on 2021-12-07
+		},
+	)
+	if err != nil {
+		log.Warningf("updates: %s", err)
+		return
+	}
+
+	// Upgrade portmaster notifier systemd user service.
+	// Permissions only!
+	err = upgradeSystemIntegrationFile(
+		"portmaster notifier systemd user service",
+		filepath.Join(dataroot.Root().Path, portmasterNotifierServiceFilePath),
+		0o0644,
+		nil, // Do not update contents.
+		nil, // Do not update contents.
+	)
+	if err != nil {
+		log.Warningf("updates: %s", err)
+		return
+	}
+}
+
+// upgradeSystemIntegrationFile upgrades the file contents and permissions.
+// System integration files are not necessarily present and may also be
+// edited by third parties, such as the OS itself or other installers.
+// The supplied hashes must be sha256 hex-encoded.
+func upgradeSystemIntegrationFile(
+	name string,
+	filePath string,
+	fileMode fs.FileMode,
+	fileData []byte,
+	permittedUpgradeHashes []string,
+) error {
+	// Upgrade file contents.
+	if len(fileData) > 0 {
+		if err := upgradeSystemIntegrationFileContents(name, filePath, fileData, permittedUpgradeHashes); err != nil {
+			return err
+		}
+	}
+
+	// Upgrade file permissions.
+	if fileMode != 0 {
+		if err := upgradeSystemIntegrationFilePermissions(name, filePath, fileMode); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// upgradeSystemIntegrationFileContents upgrades the file contents.
+// System integration files are not necessarily present and may also be
+// edited by third parties, such as the OS itself or other installers.
+// The supplied hashes must be sha256 hex-encoded.
+func upgradeSystemIntegrationFileContents(
+	name string,
+	filePath string,
+	fileData []byte,
+	permittedUpgradeHashes []string,
+) error {
+	// Read existing file.
+	existingFileData, err := os.ReadFile(filePath)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil
+		}
+		return fmt.Errorf("failed to read %s at %s: %w", name, filePath, err)
+	}
+
+	// Check if file is already the current version.
+	existingSum := sha256.Sum256(existingFileData)
+	existingHexSum := hex.EncodeToString(existingSum[:])
+	currentSum := sha256.Sum256(fileData)
+	currentHexSum := hex.EncodeToString(currentSum[:])
+	if existingHexSum == currentHexSum {
+		log.Debugf("updates: %s at %s is up to date", name, filePath)
+		return nil
+	}
+
+	// Check if we are allowed to upgrade from the existing file.
+	if !slices.Contains[string](permittedUpgradeHashes, existingHexSum) {
+		return fmt.Errorf("%s at %s %w, as it is not a previously published version and cannot be automatically upgraded - try installing again", name, filePath, ErrRequiresManualUpgrade)
+	}
+
+	// Start with upgrade!
+
+	// Make backup of existing file.
+	err = CopyFile(filePath, filePath+backupExtension)
+	if err != nil {
+		return fmt.Errorf(
+			"failed to create backup of %s from %s to %s: %w",
+			name,
+			filePath,
+			filePath+backupExtension,
+			err,
+		)
+	}
+
+	// Open destination file for writing.
+	atomicDstFile, err := renameio.TempFile(registry.TmpDir().Path, filePath)
+	if err != nil {
+		return fmt.Errorf("failed to create tmp file to update %s at %s: %w", name, filePath, err)
+	}
+	defer atomicDstFile.Cleanup() //nolint:errcheck // ignore error for now, tmp dir will be cleaned later again anyway
+
+	// Write file.
+	_, err = io.Copy(atomicDstFile, bytes.NewReader(fileData))
+	if err != nil {
+		return err
+	}
+
+	// Finalize file.
+	err = atomicDstFile.CloseAtomicallyReplace()
+	if err != nil {
+		return fmt.Errorf("failed to finalize update of %s at %s: %w", name, filePath, err)
+	}
+
+	log.Warningf("updates: %s at %s was upgraded to %s", name, filePath, currentHexSum)
+	return nil
+}
+
+// upgradeSystemIntegrationFilePermissions upgrades the file permissions.
+// System integration files are not necessarily present and may also be
+// edited by third parties, such as the OS itself or other installers.
+func upgradeSystemIntegrationFilePermissions(
+	name string,
+	filePath string,
+	fileMode fs.FileMode,
+) error {
+	// Get current file permissions.
+	stat, err := os.Stat(filePath)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil
+		}
+		return fmt.Errorf("failed to read %s file metadata at %s: %w", name, filePath, err)
+	}
+
+	// If permissions are as expected, do nothing.
+	if stat.Mode().Perm() == fileMode {
+		return nil
+	}
+
+	// Otherwise, set correct permissions.
+	err = os.Chmod(filePath, fileMode)
+	if err != nil {
+		return fmt.Errorf("failed to update %s file permissions at %s: %w", name, filePath, err)
+	}
+
+	log.Warningf("updates: %s file permissions at %s updated to %v", name, filePath, fileMode)
+	return nil
+}

updates/upgrader.go

@@ -66,15 +66,21 @@ func upgrader(_ context.Context, _ interface{}) error {
 	binBaseName := strings.Split(filepath.Base(os.Args[0]), "_")[0]
 	switch binBaseName {
 	case "portmaster-core":
+		// Notify about upgrade.
 		if err := upgradeCoreNotify(); err != nil {
 			log.Warningf("updates: failed to notify about core upgrade: %s", err)
 		}
 
+		// Fix chrome sandbox permissions.
 		if err := helper.EnsureChromeSandboxPermissions(registry); err != nil {
 			log.Warningf("updates: failed to handle electron upgrade: %s", err)
 		}
 
+		// Upgrade system integration.
+		upgradeSystemIntegration()
+
 	case "spn-hub":
+		// Trigger upgrade procedure.
 		if err := upgradeHub(); err != nil {
 			log.Warningf("updates: failed to initiate hub upgrade: %s", err)
 		}
@@ -213,7 +219,7 @@ func upgradePortmasterStart() error {
 
 	// update portmaster-start in data root
 	rootPmStartPath := filepath.Join(dataroot.Root().Path, filename)
-	err := upgradeFile(rootPmStartPath, pmCtrlUpdate)
+	err := upgradeBinary(rootPmStartPath, pmCtrlUpdate)
 	if err != nil {
 		return err
 	}
@@ -278,7 +284,7 @@ func warnOnIncorrectParentPath() {
 	}
 }
 
-func upgradeFile(fileToUpgrade string, file *updater.File) error {
+func upgradeBinary(fileToUpgrade string, file *updater.File) error {
 	fileExists := false
 	_, err := os.Stat(fileToUpgrade)
 	if err == nil {
@@ -295,7 +301,7 @@ func upgradeFile(fileToUpgrade string, file *updater.File) error {
 			// abort if version matches
 			currentVersion = strings.Trim(strings.TrimSpace(string(out)), "*")
 			if currentVersion == file.Version() {
-				log.Tracef("updates: %s is already v%s", fileToUpgrade, file.Version())
+				log.Debugf("updates: %s is already v%s", fileToUpgrade, file.Version())
 				// already up to date!
 				return nil
 			}
@@ -306,7 +312,7 @@ func upgradeFile(fileToUpgrade string, file *updater.File) error {
 
 		// test currentVersion for sanity
 		if !rawVersionRegex.MatchString(currentVersion) {
-			log.Tracef("updates: version string returned by %s is invalid: %s", fileToUpgrade, currentVersion)
+			log.Debugf("updates: version string returned by %s is invalid: %s", fileToUpgrade, currentVersion)
 		}
 
 		// try removing old version