[service] Fix file permission in the updates

base/database/main.go

@@ -3,7 +3,6 @@ package database
 import (
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 
 	"github.com/safing/portmaster/base/utils"
@@ -24,15 +23,11 @@ func Initialize(databasesRootDir string) error {
 	if initialized.SetToIf(false, true) {
 		rootDir = databasesRootDir
 
-		err := os.MkdirAll(rootDir, 0o0700)
+		// ensure root and databases dirs
+		err := utils.EnsureDirectory(rootDir, utils.AdminOnlyExecPermission)
 		if err != nil {
 			return fmt.Errorf("failed to create/check database dir %q: %w", rootDir, err)
 		}
-		// ensure root and databases dirs
-		err = utils.EnsureDirectory(rootDir, utils.AdminOnlyPermission)
-		if err != nil {
-			return fmt.Errorf("could not set permissions to database directory (%s): %w", rootDir, err)
-		}
 
 		return nil
 	}
@@ -64,13 +59,9 @@ func getLocation(name, storageType string) (string, error) {
 	location := filepath.Join(rootDir, name, storageType)
 
 	// Make sure location exists.
-	err := os.MkdirAll(location, 0o0700)
+	err := utils.EnsureDirectory(location, utils.AdminOnlyExecPermission)
 	if err != nil {
 		return "", fmt.Errorf("failed to create/check database dir %q: %w", location, err)
 	}
-	err = utils.EnsureDirectory(location, utils.AdminOnlyPermission)
-	if err != nil {
-		return "", fmt.Errorf("could not set permissions to directory (%s): %w", location, err)
-	}
 	return location, nil
 }

base/utils/fs.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"log/slog"
 	"os"
 	"runtime"
 )
@@ -13,6 +14,10 @@ const isWindows = runtime.GOOS == "windows"
 // EnsureDirectory ensures that the given directory exists and that is has the given permissions set.
 // If path is a file, it is deleted and a directory created.
 func EnsureDirectory(path string, perm FSPermission) error {
+	if !perm.IsExecPermission() {
+		slog.Warn("utils: setting not executable permission for directory", "dir", path)
+	}
+
 	// open path
 	f, err := os.Stat(path)
 	if err == nil {
@@ -21,10 +26,10 @@ func EnsureDirectory(path string, perm FSPermission) error {
 			// directory exists, check permissions
 			if isWindows {
 				// Ignore windows permission error. For none admin users it will always fail.
-				_ = SetDirPermission(path, perm)
+				_ = SetFilePermission(path, perm)
 				return nil
-			} else if f.Mode().Perm() != perm.AsUnixDirExecPermission() {
+			} else if f.Mode().Perm() != perm.AsUnixPermission() {
-				return SetDirPermission(path, perm)
+				return SetFilePermission(path, perm)
 			}
 			return nil
 		}
@@ -35,12 +40,12 @@ func EnsureDirectory(path string, perm FSPermission) error {
 	}
 	// file does not exist (or has been deleted)
 	if err == nil || errors.Is(err, fs.ErrNotExist) {
-		err = os.MkdirAll(path, perm.AsUnixDirExecPermission())
+		err = os.MkdirAll(path, perm.AsUnixPermission())
 		if err != nil {
 			return fmt.Errorf("could not create dir %s: %w", path, err)
 		}
 		// Set permissions.
-		err = SetDirPermission(path, perm)
+		err = SetFilePermission(path, perm)
 		// Ignore windows permission error. For none admin users it will always fail.
 		if !isWindows {
 			return err

base/utils/permissions.go

@@ -4,17 +4,7 @@ package utils
 
 import "os"
 
-// SetDirPermission sets the permission of a directory.
+// SetFilePermission sets the permission of a file or directory.
-func SetDirPermission(path string, perm FSPermission) error {
-	return os.Chmod(path, perm.AsUnixDirExecPermission())
-}
-
-// SetExecPermission sets the permission of an executable file.
-func SetExecPermission(path string, perm FSPermission) error {
-	return SetDirPermission(path, perm)
-}
-
-// SetFilePermission sets the permission of a non executable file.
 func SetFilePermission(path string, perm FSPermission) error {
-	return os.Chmod(path, perm.AsUnixFilePermission())
+	return os.Chmod(path, perm.AsUnixPermission())
 }

base/utils/permissions_windows.go

@@ -31,22 +31,10 @@ func init() {
 	}
 }
 
-// SetDirPermission sets the permission of a directory.
+// SetFilePermission sets the permission of a file or directory.
-func SetDirPermission(path string, perm FSPermission) error {
+func SetFilePermission(path string, perm FSPermission) error {
-	SetFilePermission(path, perm)
-	return nil
-}
-
-// SetExecPermission sets the permission of an executable file.
-func SetExecPermission(path string, perm FSPermission) error {
-	SetFilePermission(path, perm)
-	return nil
-}
-
-// SetFilePermission sets the permission of a non executable file.
-func SetFilePermission(path string, perm FSPermission) {
 	switch perm {
-	case AdminOnlyPermission:
+	case AdminOnlyPermission, AdminOnlyExecPermission:
 		// Set only admin rights, remove all others.
 		acl.Apply(
 			path,
@@ -55,7 +43,7 @@ func SetFilePermission(path string, perm FSPermission) {
 			acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, systemSID),
 			acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, adminsSID),
 		)
-	case PublicReadPermission:
+	case PublicReadPermission, PublicReadExecPermission:
 		// Set admin rights and read/execute rights for users, remove all others.
 		acl.Apply(
 			path,
@@ -65,7 +53,7 @@ func SetFilePermission(path string, perm FSPermission) {
 			acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, adminsSID),
 			acl.GrantSid(windows.GENERIC_READ|windows.GENERIC_EXECUTE, usersSID),
 		)
-	case PublicWritePermission:
+	case PublicWritePermission, PublicWriteExecPermission:
 		// Set full control to admin and regular users. Guest users will not have access.
 		acl.Apply(
 			path,
@@ -76,4 +64,5 @@ func SetFilePermission(path string, perm FSPermission) {
 			acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, usersSID),
 		)
 	}
+	return nil
 }

base/utils/structure.go

@@ -12,36 +12,39 @@ type FSPermission uint8
 
 const (
 	AdminOnlyPermission FSPermission = iota
+	AdminOnlyExecPermission
 	PublicReadPermission
+	PublicReadExecPermission
 	PublicWritePermission
+	PublicWriteExecPermission
 )
 
 // AsUnixDirExecPermission return the corresponding unix permission for a directory or executable.
-func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
+func (perm FSPermission) AsUnixPermission() fs.FileMode {
 	switch perm {
 	case AdminOnlyPermission:
+		return 0o600
+	case AdminOnlyExecPermission:
 		return 0o700
 	case PublicReadPermission:
+		return 0o644
+	case PublicReadExecPermission:
 		return 0o755
 	case PublicWritePermission:
+		return 0o666
+	case PublicWriteExecPermission:
 		return 0o777
 	}
 
 	return 0
 }
 
-// AsUnixFilePermission return the corresponding unix permission for a regular file.
+func (perm FSPermission) IsExecPermission() bool {
-func (perm FSPermission) AsUnixFilePermission() fs.FileMode {
 	switch perm {
-	case AdminOnlyPermission:
+	case AdminOnlyExecPermission, PublicReadExecPermission, PublicWriteExecPermission:
-		return 0o600
+		return true
-	case PublicReadPermission:
-		return 0o644
-	case PublicWritePermission:
-		return 0o666
 	}
-
+	return false
-	return 0
 }
 
 // DirStructure represents a directory structure with permissions that should be enforced.

service/netquery/database.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"os"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -129,12 +128,9 @@ type (
 // handed over to SQLite.
 func New(dbPath string) (*Database, error) {
 	historyParentDir := filepath.Join(module.instance.DataDir(), "databases")
-	if err := os.MkdirAll(historyParentDir, 0o0700); err != nil {
+	err := utils.EnsureDirectory(historyParentDir, utils.AdminOnlyExecPermission)
-		return nil, fmt.Errorf("failed to ensure database directory exists: %w", err)
-	}
-	err := utils.EnsureDirectory(historyParentDir, utils.AdminOnlyPermission)
 	if err != nil {
-		return nil, fmt.Errorf("failed to set permission to folder %s: %w", historyParentDir, err)
+		return nil, fmt.Errorf("failed to ensure database directory exists: %w", err)
 	}
 
 	// Get file location of history database.
@@ -231,12 +227,9 @@ func (db *Database) Close() error {
 // VacuumHistory rewrites the history database in order to purge deleted records.
 func VacuumHistory(ctx context.Context) (err error) {
 	historyParentDir := filepath.Join(module.instance.DataDir(), "databases")
-	if err := os.MkdirAll(historyParentDir, 0o0700); err != nil {
+	err = utils.EnsureDirectory(historyParentDir, utils.AdminOnlyExecPermission)
-		return fmt.Errorf("failed to ensure database directory exists: %w", err)
-	}
-	err = utils.EnsureDirectory(historyParentDir, utils.AdminOnlyPermission)
 	if err != nil {
-		return fmt.Errorf("failed to set permission to folder %s: %w", historyParentDir, err)
+		return fmt.Errorf("failed to ensure database directory exists: %w", err)
 	}
 
 	// Get file location of history database.

service/profile/module.go

@@ -3,7 +3,6 @@ package profile
 import (
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 	"sync/atomic"
 
@@ -68,19 +67,15 @@ func prep() error {
 
 	// Setup icon storage location.
 	databaseDir := filepath.Join(module.instance.DataDir(), "databases")
+	// Ensure folder existents and permission
+	err := utils.EnsureDirectory(databaseDir, utils.AdminOnlyExecPermission)
+	if err != nil {
+		return fmt.Errorf("failed to ensure directory existence %s: %w", databaseDir, err)
+	}
 	iconsDir := filepath.Join(databaseDir, "icons")
-	if err := os.MkdirAll(iconsDir, 0o0700); err != nil {
+	err = utils.EnsureDirectory(iconsDir, utils.AdminOnlyExecPermission)
-		return fmt.Errorf("failed to create/check icons directory: %w", err)
-	}
-
-	// Ensure folder permissions
-	err := utils.EnsureDirectory(databaseDir, utils.AdminOnlyPermission)
 	if err != nil {
-		return fmt.Errorf("failed to set permission to folder %s: %w", databaseDir, err)
+		return fmt.Errorf("failed to ensure directory existence %s: %w", iconsDir, err)
-	}
-	err = utils.EnsureDirectory(iconsDir, utils.AdminOnlyPermission)
-	if err != nil {
-		return fmt.Errorf("failed to set permission to folder %s: %w", iconsDir, err)
 	}
 
 	binmeta.ProfileIconStoragePath = iconsDir

service/ui/module.go

@@ -37,7 +37,7 @@ func start() error {
 	}
 
 	// Ensure directory permission
-	err = utils.EnsureDirectory(execDir, utils.PublicWritePermission)
+	err = utils.EnsureDirectory(execDir, utils.PublicWriteExecPermission)
 	if err != nil {
 		log.Warningf("ui: failed to set permissions to directory %s: %s", execDir, err)
 	}

service/updates/downloader.go

@@ -16,6 +16,7 @@ import (
 	"path/filepath"
 
 	"github.com/safing/portmaster/base/log"
+	"github.com/safing/portmaster/base/utils"
 )
 
 type Downloader struct {
@@ -37,7 +38,7 @@ func NewDownloader(u *Updater, indexURLs []string) *Downloader {
 
 func (d *Downloader) updateIndex(ctx context.Context) error {
 	// Make sure dir exists.
-	err := os.MkdirAll(d.u.cfg.DownloadDirectory, defaultDirMode)
+	err := os.MkdirAll(d.u.cfg.DownloadDirectory, utils.PublicReadExecPermission.AsUnixPermission())
 	if err != nil {
 		return fmt.Errorf("create download directory: %s", d.u.cfg.DownloadDirectory)
 	}
@@ -65,7 +66,7 @@ func (d *Downloader) updateIndex(ctx context.Context) error {
 
 	// Write the index into a file.
 	indexFilepath := filepath.Join(d.u.cfg.DownloadDirectory, d.u.cfg.IndexFile)
-	err = os.WriteFile(indexFilepath, indexData, defaultFileMode)
+	err = os.WriteFile(indexFilepath, indexData, utils.PublicReadExecPermission.AsUnixPermission())
 	if err != nil {
 		return fmt.Errorf("write index file: %w", err)
 	}
@@ -130,7 +131,7 @@ func (d *Downloader) gatherExistingFiles(dir string) error {
 
 func (d *Downloader) downloadArtifacts(ctx context.Context) error {
 	// Make sure dir exists.
-	err := os.MkdirAll(d.u.cfg.DownloadDirectory, defaultDirMode)
+	err := os.MkdirAll(d.u.cfg.DownloadDirectory, utils.PublicReadExecPermission.AsUnixPermission())
 	if err != nil {
 		return fmt.Errorf("create download directory: %s", d.u.cfg.DownloadDirectory)
 	}
@@ -176,7 +177,7 @@ artifacts:
 
 		// Write artifact to temporary file.
 		tmpFilename := dstFilePath + ".download"
-		err = os.WriteFile(tmpFilename, artifactData, artifact.GetFileMode())
+		err = os.WriteFile(tmpFilename, artifactData, artifact.GetFileMode().AsUnixPermission())
 		if err != nil {
 			return fmt.Errorf("write %s to temp file: %w", artifact.Filename, err)
 		}

service/updates/index.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -18,6 +17,7 @@ import (
 
 	"github.com/safing/jess"
 	"github.com/safing/jess/filesig"
+	"github.com/safing/portmaster/base/utils"
 )
 
 // MaxUnpackSize defines the maximum size that is allowed to be unpacked.
@@ -41,17 +41,12 @@ type Artifact struct {
 }
 
 // GetFileMode returns the required filesystem permission for the artifact.
-func (a *Artifact) GetFileMode() os.FileMode {
+func (a *Artifact) GetFileMode() utils.FSPermission {
-	// Special case for portmaster ui. Should be able to be executed from the regular user
-	if a.Platform == currentPlatform && a.Filename == "portmaster" {
-		return executableUIFileMode
-	}
-
 	if a.Platform == currentPlatform {
-		return executableFileMode
+		return utils.PublicReadExecPermission
 	}
 
-	return defaultFileMode
+	return utils.PublicReadPermission
 }
 
 // Path returns the absolute path to the local file.
@@ -338,7 +333,7 @@ func checkSHA256Sum(fileData []byte, sha256sum string) error {
 
 // copyAndCheckSHA256Sum copies the file from src to dst and check the sha256 sum.
 // As a special case, if the sha256sum is not given, it is not checked.
-func copyAndCheckSHA256Sum(src, dst, sha256sum string, fileMode fs.FileMode) error {
+func copyAndCheckSHA256Sum(src, dst, sha256sum string, filePermission utils.FSPermission) error {
 	// Check expected hash.
 	var expectedDigest []byte
 	if sha256sum != "" {
@@ -367,7 +362,7 @@ func copyAndCheckSHA256Sum(src, dst, sha256sum string, fileMode fs.FileMode) err
 
 	// Write to temporary file.
 	tmpDst := dst + ".copy"
-	err = os.WriteFile(tmpDst, fileData, fileMode)
+	err = os.WriteFile(tmpDst, fileData, filePermission.AsUnixPermission())
 	if err != nil {
 		return fmt.Errorf("write temp dst file: %w", err)
 	}
@@ -377,6 +372,7 @@ func copyAndCheckSHA256Sum(src, dst, sha256sum string, fileMode fs.FileMode) err
 	if err != nil {
 		return fmt.Errorf("rename dst file after write: %w", err)
 	}
+	utils.SetFilePermission(dst, filePermission)
 
 	return nil
 }

service/updates/upgrade.go

@@ -3,20 +3,13 @@ package updates
 import (
 	"errors"
 	"fmt"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"slices"
 	"strings"
 
 	"github.com/safing/portmaster/base/log"
-)
+	"github.com/safing/portmaster/base/utils"
-
-const (
-	defaultFileMode      = os.FileMode(0o0644)
-	executableFileMode   = os.FileMode(0o0744)
-	executableUIFileMode = os.FileMode(0o0755)
-	defaultDirMode       = os.FileMode(0o0755)
 )
 
 func (u *Updater) upgrade(downloader *Downloader, ignoreVersion bool) error {
@@ -55,7 +48,7 @@ func (u *Updater) upgradeMoveFiles(downloader *Downloader) error {
 
 	// Reset purge directory, so that we can do a clean rollback later.
 	_ = os.RemoveAll(u.cfg.PurgeDirectory)
-	err := os.MkdirAll(u.cfg.PurgeDirectory, defaultDirMode)
+	err := utils.EnsureDirectory(u.cfg.PurgeDirectory, utils.PublicReadExecPermission)
 	if err != nil {
 		return fmt.Errorf("failed to create purge directory: %w", err)
 	}
@@ -69,7 +62,7 @@ func (u *Updater) upgradeMoveFiles(downloader *Downloader) error {
 		if !errors.Is(err, os.ErrNotExist) {
 			return fmt.Errorf("read current directory: %w", err)
 		}
-		err = os.MkdirAll(u.cfg.Directory, defaultDirMode)
+		err := utils.EnsureDirectory(u.cfg.PurgeDirectory, utils.PublicReadExecPermission)
 		if err != nil {
 			return fmt.Errorf("create current directory: %w", err)
 		}
@@ -84,7 +77,7 @@ func (u *Updater) upgradeMoveFiles(downloader *Downloader) error {
 			// Otherwise, move file to purge dir.
 			src := filepath.Join(u.cfg.Directory, file.Name())
 			dst := filepath.Join(u.cfg.PurgeDirectory, file.Name())
-			err := u.moveFile(src, dst, "", file.Type().Perm())
+			err := u.moveFile(src, dst, "", utils.PublicReadPermission)
 			if err != nil {
 				return fmt.Errorf("failed to move current file %s to purge dir: %w", file.Name(), err)
 			}
@@ -95,7 +88,7 @@ func (u *Updater) upgradeMoveFiles(downloader *Downloader) error {
 	log.Debugf("updates/%s: installing the new version (v%s from %s)", u.cfg.Name, downloader.index.Version, downloader.index.Published)
 	src := filepath.Join(u.cfg.DownloadDirectory, u.cfg.IndexFile)
 	dst := filepath.Join(u.cfg.Directory, u.cfg.IndexFile)
-	err = u.moveFile(src, dst, "", defaultFileMode)
+	err = u.moveFile(src, dst, "", utils.PublicReadPermission)
 	if err != nil {
 		return fmt.Errorf("failed to move index file to %s: %w", dst, err)
 	}
@@ -120,17 +113,18 @@ func (u *Updater) upgradeMoveFiles(downloader *Downloader) error {
 }
 
 // moveFile moves a file and falls back to copying if it fails.
-func (u *Updater) moveFile(currentPath, newPath string, sha256sum string, fileMode fs.FileMode) error {
+func (u *Updater) moveFile(currentPath, newPath string, sha256sum string, filePermission utils.FSPermission) error {
 	// Try to simply move file.
 	err := os.Rename(currentPath, newPath)
 	if err == nil {
 		// Moving was successful, return.
+		utils.SetFilePermission(newPath, filePermission)
 		return nil
 	}
 	log.Tracef("updates/%s: failed to move to %q, falling back to copy+delete: %s", u.cfg.Name, newPath, err)
 
 	// Copy and check the checksum while we are at it.
-	err = copyAndCheckSHA256Sum(currentPath, newPath, sha256sum, fileMode)
+	err = copyAndCheckSHA256Sum(currentPath, newPath, sha256sum, filePermission)
 	if err != nil {
 		return fmt.Errorf("move failed, copy+delete fallback failed: %w", err)
 	}
@@ -150,7 +144,7 @@ func (u *Updater) recoverFromFailedUpgrade() error {
 	for _, file := range files {
 		purgedFile := filepath.Join(u.cfg.PurgeDirectory, file.Name())
 		activeFile := filepath.Join(u.cfg.Directory, file.Name())
-		err := u.moveFile(purgedFile, activeFile, "", file.Type().Perm())
+		err := u.moveFile(purgedFile, activeFile, "", utils.PublicReadPermission)
 		if err != nil {
 			// Only warn and continue to recover as many files as possible.
 			log.Warningf("updates/%s: failed to roll back file %s: %s", u.cfg.Name, file.Name(), err)