From 22253c4e9e53918d9675dff5ffed67b3baa2304f Mon Sep 17 00:00:00 2001
From: Vladimir Stoilov <vladimir@safing.io>
Date: Fri, 6 Dec 2024 12:00:20 +0200
Subject: [PATCH 1/4] [service] Fix windows permissions
---
base/config/main.go | 4 +--
base/database/main.go | 6 ++--
base/database/storage/fstree/fstree.go | 7 ++--
base/dataroot/root.go | 3 +-
base/updater/fetch.go | 17 +++-------
base/updater/registry.go | 2 +-
base/utils/fs.go | 19 +++++------
base/utils/permissions.go | 20 ++++++++++++
base/utils/permissions_windows.go | 35 ++++++++++++++++++++
base/utils/structure.go | 44 +++++++++++++++++++++++---
cmds/portmaster-start/dirs.go | 3 +-
cmds/portmaster-start/main.go | 6 ++--
service/core/base/global.go | 3 +-
service/netquery/database.go | 5 +--
service/profile/module.go | 3 +-
service/ui/module.go | 3 +-
service/updates/main.go | 3 +-
service/updates/upgrader.go | 12 +++----
18 files changed, 138 insertions(+), 57 deletions(-)
create mode 100644 base/utils/permissions.go
create mode 100644 base/utils/permissions_windows.go
diff --git a/base/config/main.go b/base/config/main.go
index 0ed0b7e6..c737dc2f 100644
--- a/base/config/main.go
+++ b/base/config/main.go
@@ -144,9 +144,9 @@ func InitializeUnitTestDataroot(testName string) (string, error) {
return "", fmt.Errorf("failed to make tmp dir: %w", err)
}
- ds := utils.NewDirStructure(basePath, 0o0755)
+ ds := utils.NewDirStructure(basePath, utils.PublicReadPermission)
SetDataRoot(ds)
- err = dataroot.Initialize(basePath, 0o0755)
+ err = dataroot.Initialize(basePath, utils.PublicReadPermission)
if err != nil {
return "", fmt.Errorf("failed to initialize dataroot: %w", err)
}
diff --git a/base/database/main.go b/base/database/main.go
index f84a0108..9c9aa1ed 100644
--- a/base/database/main.go
+++ b/base/database/main.go
@@ -25,7 +25,7 @@ var (
// InitializeWithPath initializes the database at the specified location using a path.
func InitializeWithPath(dirPath string) error {
- return Initialize(utils.NewDirStructure(dirPath, 0o0755))
+ return Initialize(utils.NewDirStructure(dirPath, utils.PublicReadPermission))
}
// Initialize initializes the database at the specified location using a dir structure.
@@ -34,7 +34,7 @@ func Initialize(dirStructureRoot *utils.DirStructure) error {
rootStructure = dirStructureRoot
// ensure root and databases dirs
- databasesStructure = rootStructure.ChildDir(databasesSubDir, 0o0700)
+ databasesStructure = rootStructure.ChildDir(databasesSubDir, utils.AdminOnlyPermission)
err := databasesStructure.Ensure()
if err != nil {
return fmt.Errorf("could not create/open database directory (%s): %w", rootStructure.Path, err)
@@ -67,7 +67,7 @@ func Shutdown() (err error) {
// getLocation returns the storage location for the given name and type.
func getLocation(name, storageType string) (string, error) {
- location := databasesStructure.ChildDir(name, 0o0700).ChildDir(storageType, 0o0700)
+ location := databasesStructure.ChildDir(name, utils.AdminOnlyPermission).ChildDir(storageType, utils.AdminOnlyPermission)
// check location
err := location.Ensure()
if err != nil {
diff --git a/base/database/storage/fstree/fstree.go b/base/database/storage/fstree/fstree.go
index 7965439a..0b4f175c 100644
--- a/base/database/storage/fstree/fstree.go
+++ b/base/database/storage/fstree/fstree.go
@@ -289,11 +289,8 @@ func writeFile(filename string, data []byte, perm os.FileMode) error {
defer t.Cleanup() //nolint:errcheck
// Set permissions before writing data, in case the data is sensitive.
- if onWindows {
- err = acl.Chmod(filename, perm)
- } else {
- err = t.Chmod(perm)
- }
+ // TODO(vladimir): to set permissions on windows we need the full path of the file.
+ err = t.Chmod(perm)
if err != nil {
return err
}
diff --git a/base/dataroot/root.go b/base/dataroot/root.go
index 296b342f..75805255 100644
--- a/base/dataroot/root.go
+++ b/base/dataroot/root.go
@@ -2,7 +2,6 @@ package dataroot
import (
"errors"
- "os"
"github.com/safing/portmaster/base/utils"
)
@@ -10,7 +9,7 @@ import (
var root *utils.DirStructure
// Initialize initializes the data root directory.
-func Initialize(rootDir string, perm os.FileMode) error {
+func Initialize(rootDir string, perm utils.FSPermission) error {
if root != nil {
return errors.New("already initialized")
}
diff --git a/base/updater/fetch.go b/base/updater/fetch.go
index 150037ed..f7f373dc 100644
--- a/base/updater/fetch.go
+++ b/base/updater/fetch.go
@@ -14,10 +14,10 @@ import (
"path/filepath"
"time"
- "github.com/hectane/go-acl"
"github.com/safing/jess/filesig"
"github.com/safing/jess/lhash"
"github.com/safing/portmaster/base/log"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/base/utils/renameio"
)
@@ -137,17 +137,10 @@ func (reg *ResourceRegistry) fetchFile(ctx context.Context, client *http.Client,
return fmt.Errorf("%s: failed to finalize file %s: %w", reg.Name, rv.storagePath(), err)
}
// set permissions
- if onWindows {
- err = acl.Chmod(rv.storagePath(), 0o0755)
- if err != nil {
- log.Warningf("%s: failed to set permissions on downloaded file %s: %s", reg.Name, rv.storagePath(), err)
- }
- } else {
- // TODO: only set executable files to 0755, set other to 0644
- err = os.Chmod(rv.storagePath(), 0o0755) //nolint:gosec // See TODO above.
- if err != nil {
- log.Warningf("%s: failed to set permissions on downloaded file %s: %s", reg.Name, rv.storagePath(), err)
- }
+ // TODO: distinguish between executable and non executable files.
+ err = utils.SetExecPermission(rv.storagePath(), utils.PublicReadPermission)
+ if err != nil {
+ log.Warningf("%s: failed to set permissions on downloaded file %s: %s", reg.Name, rv.storagePath(), err)
}
log.Debugf("%s: fetched %s and stored to %s", reg.Name, downloadURL, rv.storagePath())
diff --git a/base/updater/registry.go b/base/updater/registry.go
index 8deda74e..4450fe98 100644
--- a/base/updater/registry.go
+++ b/base/updater/registry.go
@@ -98,7 +98,7 @@ func (reg *ResourceRegistry) Initialize(storageDir *utils.DirStructure) error {
// initialize private attributes
reg.storageDir = storageDir
- reg.tmpDir = storageDir.ChildDir("tmp", 0o0700)
+ reg.tmpDir = storageDir.ChildDir("tmp", utils.AdminOnlyPermission)
reg.resources = make(map[string]*Resource)
if reg.state == nil {
reg.state = &RegistryState{}
diff --git a/base/utils/fs.go b/base/utils/fs.go
index bb59960f..6ede989d 100644
--- a/base/utils/fs.go
+++ b/base/utils/fs.go
@@ -6,15 +6,13 @@ import (
"io/fs"
"os"
"runtime"
-
- "github.com/hectane/go-acl"
)
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 os.FileMode) error {
+func EnsureDirectory(path string, perm FSPermission) error {
// open path
f, err := os.Stat(path)
if err == nil {
@@ -23,10 +21,10 @@ func EnsureDirectory(path string, perm os.FileMode) error {
// directory exists, check permissions
if isWindows {
// Ignore windows permission error. For none admin users it will always fail.
- acl.Chmod(path, perm)
+ SetDirPermission(path, perm)
return nil
- } else if f.Mode().Perm() != perm {
- return os.Chmod(path, perm)
+ } else if f.Mode().Perm() != perm.AsUnixDirExecPermission() {
+ return SetDirPermission(path, perm)
}
return nil
}
@@ -37,17 +35,16 @@ func EnsureDirectory(path string, perm os.FileMode) error {
}
// file does not exist (or has been deleted)
if err == nil || errors.Is(err, fs.ErrNotExist) {
- err = os.Mkdir(path, perm)
+ err = os.Mkdir(path, perm.AsUnixDirExecPermission())
if err != nil {
return fmt.Errorf("could not create dir %s: %w", path, err)
}
+ // Set windows permissions. Linux permission where already set with creation.
if isWindows {
// Ignore windows permission error. For none admin users it will always fail.
- acl.Chmod(path, perm)
- return nil
- } else {
- return os.Chmod(path, perm)
+ SetDirPermission(path, perm)
}
+ return nil
}
// other error opening path
return fmt.Errorf("failed to access %s: %w", path, err)
diff --git a/base/utils/permissions.go b/base/utils/permissions.go
new file mode 100644
index 00000000..d7690724
--- /dev/null
+++ b/base/utils/permissions.go
@@ -0,0 +1,20 @@
+//go:build !windows
+
+package utils
+
+import "os"
+
+// SetDirPermission sets the permission of a 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())
+}
diff --git a/base/utils/permissions_windows.go b/base/utils/permissions_windows.go
new file mode 100644
index 00000000..ac48c21a
--- /dev/null
+++ b/base/utils/permissions_windows.go
@@ -0,0 +1,35 @@
+//go:build windows
+
+package utils
+
+import (
+ "github.com/hectane/go-acl"
+ "golang.org/x/sys/windows"
+)
+
+func SetDirPermission(path string, perm FSPermission) error {
+ setWindowsFilePermissions(path, perm)
+ return nil
+}
+
+// SetExecPermission sets the permission of an executable file.
+func SetExecPermission(path string, perm FSPermission) error {
+ return SetDirPermission(path, perm)
+}
+
+func setWindowsFilePermissions(path string, perm FSPermission) {
+ switch perm {
+ case AdminOnlyPermission:
+ // Set only admin rights, remove all others.
+ acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
+ case PublicReadPermission:
+ // Set admin rights and read/execute rights for users, remove all others.
+ acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
+ acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_EXECUTE, "Users"))
+ acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_READ, "Users"))
+ case PublicWritePermission:
+ // Set full control to admin and regular users. Guest users will not have access.
+ acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
+ acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Users"))
+ }
+}
diff --git a/base/utils/structure.go b/base/utils/structure.go
index 5a50d97e..e33a920f 100644
--- a/base/utils/structure.go
+++ b/base/utils/structure.go
@@ -2,25 +2,61 @@ package utils
import (
"fmt"
- "os"
+ "io/fs"
"path/filepath"
"strings"
"sync"
)
+type FSPermission uint8
+
+const (
+ AdminOnlyPermission FSPermission = iota
+ PublicReadPermission
+ PublicWritePermission
+)
+
+// AsUnixDirPermission return the corresponding unix permission for a directory or executable.
+func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
+ switch perm {
+ case AdminOnlyPermission:
+ return 0o700
+ case PublicReadPermission:
+ return 0o755
+ case PublicWritePermission:
+ return 0o777
+ }
+
+ return 0
+}
+
+// AsUnixDirPermission return the corresponding unix permission for a regular file.
+func (perm FSPermission) AsUnixFilePermission() fs.FileMode {
+ switch perm {
+ case AdminOnlyPermission:
+ return 0o600
+ case PublicReadPermission:
+ return 0o655
+ case PublicWritePermission:
+ return 0o666
+ }
+
+ return 0
+}
+
// DirStructure represents a directory structure with permissions that should be enforced.
type DirStructure struct {
sync.Mutex
Path string
Dir string
- Perm os.FileMode
+ Perm FSPermission
Parent *DirStructure
Children map[string]*DirStructure
}
// NewDirStructure returns a new DirStructure.
-func NewDirStructure(path string, perm os.FileMode) *DirStructure {
+func NewDirStructure(path string, perm FSPermission) *DirStructure {
return &DirStructure{
Path: path,
Perm: perm,
@@ -29,7 +65,7 @@ func NewDirStructure(path string, perm os.FileMode) *DirStructure {
}
// ChildDir adds a new child DirStructure and returns it. Should the child already exist, the existing child is returned and the permissions are updated.
-func (ds *DirStructure) ChildDir(dirName string, perm os.FileMode) (child *DirStructure) {
+func (ds *DirStructure) ChildDir(dirName string, perm FSPermission) (child *DirStructure) {
ds.Lock()
defer ds.Unlock()
diff --git a/cmds/portmaster-start/dirs.go b/cmds/portmaster-start/dirs.go
index e327963f..a95c500b 100644
--- a/cmds/portmaster-start/dirs.go
+++ b/cmds/portmaster-start/dirs.go
@@ -5,6 +5,7 @@ import (
"log"
"os"
+ "github.com/safing/portmaster/base/utils"
"github.com/spf13/cobra"
)
@@ -24,7 +25,7 @@ var cleanStructureCmd = &cobra.Command{
}
func cleanAndEnsureExecDir() error {
- execDir := dataRoot.ChildDir("exec", 0o777)
+ execDir := dataRoot.ChildDir("exec", utils.PublicWritePermission)
// Clean up and remove exec dir.
err := os.RemoveAll(execDir.Path)
diff --git a/cmds/portmaster-start/main.go b/cmds/portmaster-start/main.go
index f764dfbf..d13a4bd6 100644
--- a/cmds/portmaster-start/main.go
+++ b/cmds/portmaster-start/main.go
@@ -179,14 +179,14 @@ func configureRegistry(mustLoadIndex bool) error {
// Remove left over quotes.
dataDir = strings.Trim(dataDir, `\"`)
// Initialize data root.
- err := dataroot.Initialize(dataDir, 0o0755)
+ err := dataroot.Initialize(dataDir, utils.PublicReadPermission)
if err != nil {
return fmt.Errorf("failed to initialize data root: %w", err)
}
dataRoot = dataroot.Root()
// Initialize registry.
- err = registry.Initialize(dataRoot.ChildDir("updates", 0o0755))
+ err = registry.Initialize(dataRoot.ChildDir("updates", utils.PublicReadPermission))
if err != nil {
return err
}
@@ -196,7 +196,7 @@ func configureRegistry(mustLoadIndex bool) error {
func ensureLoggingDir() error {
// set up logs root
- logsRoot = dataRoot.ChildDir("logs", 0o0777)
+ logsRoot = dataRoot.ChildDir("logs", utils.PublicWritePermission)
err := logsRoot.Ensure()
if err != nil {
return fmt.Errorf("failed to initialize logs root (%q): %w", logsRoot.Path, err)
diff --git a/service/core/base/global.go b/service/core/base/global.go
index 3b1cc82f..fa67048f 100644
--- a/service/core/base/global.go
+++ b/service/core/base/global.go
@@ -8,6 +8,7 @@ import (
"github.com/safing/portmaster/base/api"
"github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/info"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/service/mgr"
)
@@ -54,7 +55,7 @@ func prep(instance instance) error {
}
// initialize structure
- err := dataroot.Initialize(dataDir, 0o0755)
+ err := dataroot.Initialize(dataDir, utils.PublicReadPermission)
if err != nil {
return err
}
diff --git a/service/netquery/database.go b/service/netquery/database.go
index a1cd6aea..912fe3cc 100644
--- a/service/netquery/database.go
+++ b/service/netquery/database.go
@@ -19,6 +19,7 @@ import (
"github.com/safing/portmaster/base/config"
"github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/log"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/service/netquery/orm"
"github.com/safing/portmaster/service/network"
"github.com/safing/portmaster/service/network/netutils"
@@ -127,7 +128,7 @@ type (
// Note that write connections are serialized by the Database object before being
// handed over to SQLite.
func New(dbPath string) (*Database, error) {
- historyParentDir := dataroot.Root().ChildDir("databases", 0o700)
+ historyParentDir := dataroot.Root().ChildDir("databases", utils.AdminOnlyPermission)
if err := historyParentDir.Ensure(); err != nil {
return nil, fmt.Errorf("failed to ensure database directory exists: %w", err)
}
@@ -225,7 +226,7 @@ func (db *Database) Close() error {
// VacuumHistory rewrites the history database in order to purge deleted records.
func VacuumHistory(ctx context.Context) (err error) {
- historyParentDir := dataroot.Root().ChildDir("databases", 0o700)
+ historyParentDir := dataroot.Root().ChildDir("databases", utils.AdminOnlyPermission)
if err := historyParentDir.Ensure(); err != nil {
return fmt.Errorf("failed to ensure database directory exists: %w", err)
}
diff --git a/service/profile/module.go b/service/profile/module.go
index 911ef99c..652fcd52 100644
--- a/service/profile/module.go
+++ b/service/profile/module.go
@@ -11,6 +11,7 @@ import (
"github.com/safing/portmaster/base/database/migration"
"github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/log"
+ "github.com/safing/portmaster/base/utils"
_ "github.com/safing/portmaster/service/core/base"
"github.com/safing/portmaster/service/mgr"
"github.com/safing/portmaster/service/profile/binmeta"
@@ -70,7 +71,7 @@ func prep() error {
}
// Setup icon storage location.
- iconsDir := dataroot.Root().ChildDir("databases", 0o0700).ChildDir("icons", 0o0700)
+ iconsDir := dataroot.Root().ChildDir("databases", utils.AdminOnlyPermission).ChildDir("icons", utils.AdminOnlyPermission)
if err := iconsDir.Ensure(); err != nil {
return fmt.Errorf("failed to create/check icons directory: %w", err)
}
diff --git a/service/ui/module.go b/service/ui/module.go
index 630808e5..b9f7220f 100644
--- a/service/ui/module.go
+++ b/service/ui/module.go
@@ -7,6 +7,7 @@ import (
"github.com/safing/portmaster/base/api"
"github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/log"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/service/mgr"
)
@@ -27,7 +28,7 @@ func start() error {
// may seem dangerous, but proper permission on the parent directory provide
// (some) protection.
// Processes must _never_ read from this directory.
- err := dataroot.Root().ChildDir("exec", 0o0777).Ensure()
+ err := dataroot.Root().ChildDir("exec", utils.PublicWritePermission).Ensure()
if err != nil {
log.Warningf("ui: failed to create safe exec dir: %s", err)
}
diff --git a/service/updates/main.go b/service/updates/main.go
index bb942993..c5a41d43 100644
--- a/service/updates/main.go
+++ b/service/updates/main.go
@@ -13,6 +13,7 @@ import (
"github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/log"
"github.com/safing/portmaster/base/updater"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/service/mgr"
"github.com/safing/portmaster/service/updates/helper"
)
@@ -138,7 +139,7 @@ func start() error {
}
// initialize
- err = registry.Initialize(dataroot.Root().ChildDir(updatesDirName, 0o0755))
+ err = registry.Initialize(dataroot.Root().ChildDir(updatesDirName, utils.PublicReadPermission))
if err != nil {
return err
}
diff --git a/service/updates/upgrader.go b/service/updates/upgrader.go
index 4963bced..f1d3c297 100644
--- a/service/updates/upgrader.go
+++ b/service/updates/upgrader.go
@@ -11,7 +11,6 @@ import (
"strings"
"time"
- "github.com/hectane/go-acl"
processInfo "github.com/shirou/gopsutil/process"
"github.com/tevino/abool"
@@ -21,6 +20,7 @@ import (
"github.com/safing/portmaster/base/notifications"
"github.com/safing/portmaster/base/rng"
"github.com/safing/portmaster/base/updater"
+ "github.com/safing/portmaster/base/utils"
"github.com/safing/portmaster/base/utils/renameio"
"github.com/safing/portmaster/service/mgr"
"github.com/safing/portmaster/service/updates/helper"
@@ -351,17 +351,15 @@ func upgradeBinary(fileToUpgrade string, file *updater.File) error {
// check permissions
if onWindows {
- err = acl.Chmod(fileToUpgrade, 0o0755)
- if err != nil {
- return fmt.Errorf("failed to set permissions on %s: %w", fileToUpgrade, err)
- }
+ utils.SetExecPermission(fileToUpgrade, utils.PublicReadPermission)
} else {
+ perm := utils.PublicReadPermission
info, err := os.Stat(fileToUpgrade)
if err != nil {
return fmt.Errorf("failed to get file info on %s: %w", fileToUpgrade, err)
}
- if info.Mode() != 0o0755 {
- err := os.Chmod(fileToUpgrade, 0o0755) //nolint:gosec // Set execute permissions.
+ if info.Mode() != perm.AsUnixDirExecPermission() {
+ err = utils.SetExecPermission(fileToUpgrade, perm)
if err != nil {
return fmt.Errorf("failed to set permissions on %s: %w", fileToUpgrade, err)
}
From 05a5d5e350c57b2a047a2d5c1db5d1a8c90eef82 Mon Sep 17 00:00:00 2001
From: Vladimir Stoilov <vladimir@safing.io>
Date: Fri, 6 Dec 2024 13:28:24 +0200
Subject: [PATCH 2/4] [service] Fix unit tests
---
base/database/storage/fstree/fstree.go | 1 -
base/updater/registry.go | 5 -----
base/updater/registry_test.go | 2 +-
base/utils/fs.go | 11 ++++++-----
base/utils/permissions_windows.go | 3 +++
base/utils/structure.go | 6 +++---
base/utils/structure_test.go | 26 +++++++++++++-------------
cmds/notifier/main.go | 4 ++--
cmds/updatemgr/main.go | 2 +-
cmds/updatemgr/release.go | 3 ++-
service/updates/upgrader.go | 2 +-
11 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/base/database/storage/fstree/fstree.go b/base/database/storage/fstree/fstree.go
index 0b4f175c..4b04f41d 100644
--- a/base/database/storage/fstree/fstree.go
+++ b/base/database/storage/fstree/fstree.go
@@ -15,7 +15,6 @@ import (
"strings"
"time"
- "github.com/hectane/go-acl"
"github.com/safing/portmaster/base/database/iterator"
"github.com/safing/portmaster/base/database/query"
"github.com/safing/portmaster/base/database/record"
diff --git a/base/updater/registry.go b/base/updater/registry.go
index 4450fe98..a2bf5fcd 100644
--- a/base/updater/registry.go
+++ b/base/updater/registry.go
@@ -5,7 +5,6 @@ import (
"fmt"
"os"
"path/filepath"
- "runtime"
"strings"
"sync"
@@ -13,10 +12,6 @@ import (
"github.com/safing/portmaster/base/utils"
)
-const (
- onWindows = runtime.GOOS == "windows"
-)
-
// ResourceRegistry is a registry for managing update resources.
type ResourceRegistry struct {
sync.RWMutex
diff --git a/base/updater/registry_test.go b/base/updater/registry_test.go
index a8978f68..1b409b25 100644
--- a/base/updater/registry_test.go
+++ b/base/updater/registry_test.go
@@ -20,7 +20,7 @@ func TestMain(m *testing.M) {
DevMode: true,
Online: true,
}
- err = registry.Initialize(utils.NewDirStructure(tmpDir, 0o0777))
+ err = registry.Initialize(utils.NewDirStructure(tmpDir, utils.PublicWritePermission))
if err != nil {
panic(err)
}
diff --git a/base/utils/fs.go b/base/utils/fs.go
index 6ede989d..5eb456b1 100644
--- a/base/utils/fs.go
+++ b/base/utils/fs.go
@@ -21,7 +21,7 @@ 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)
+ _ = SetDirPermission(path, perm)
return nil
} else if f.Mode().Perm() != perm.AsUnixDirExecPermission() {
return SetDirPermission(path, perm)
@@ -39,10 +39,11 @@ func EnsureDirectory(path string, perm FSPermission) error {
if err != nil {
return fmt.Errorf("could not create dir %s: %w", path, err)
}
- // Set windows permissions. Linux permission where already set with creation.
- if isWindows {
- // Ignore windows permission error. For none admin users it will always fail.
- SetDirPermission(path, perm)
+ // Set permissions.
+ err = SetDirPermission(path, perm)
+ // Ignore windows permission error. For none admin users it will always fail.
+ if !isWindows {
+ return err
}
return nil
}
diff --git a/base/utils/permissions_windows.go b/base/utils/permissions_windows.go
index ac48c21a..5f36ebb9 100644
--- a/base/utils/permissions_windows.go
+++ b/base/utils/permissions_windows.go
@@ -32,4 +32,7 @@ func setWindowsFilePermissions(path string, perm FSPermission) {
acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Users"))
}
+
+ // For completeness
+ acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "SYSTEM"))
}
diff --git a/base/utils/structure.go b/base/utils/structure.go
index e33a920f..8e1f0786 100644
--- a/base/utils/structure.go
+++ b/base/utils/structure.go
@@ -16,7 +16,7 @@ const (
PublicWritePermission
)
-// AsUnixDirPermission return the corresponding unix permission for a directory or executable.
+// AsUnixDirExecPermission return the corresponding unix permission for a directory or executable.
func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
switch perm {
case AdminOnlyPermission:
@@ -30,13 +30,13 @@ func (perm FSPermission) AsUnixDirExecPermission() fs.FileMode {
return 0
}
-// AsUnixDirPermission return the corresponding unix permission for a regular file.
+// AsUnixFilePermission return the corresponding unix permission for a regular file.
func (perm FSPermission) AsUnixFilePermission() fs.FileMode {
switch perm {
case AdminOnlyPermission:
return 0o600
case PublicReadPermission:
- return 0o655
+ return 0o644
case PublicWritePermission:
return 0o666
}
diff --git a/base/utils/structure_test.go b/base/utils/structure_test.go
index 2acfebd2..d3debcf7 100644
--- a/base/utils/structure_test.go
+++ b/base/utils/structure_test.go
@@ -13,13 +13,13 @@ func ExampleDirStructure() {
// output:
// / [755]
// /repo [777]
- // /repo/b [707]
- // /repo/b/c [750]
- // /repo/b/d [707]
- // /repo/b/d/e [707]
- // /repo/b/d/f [707]
- // /repo/b/d/f/g [707]
- // /repo/b/d/f/g/h [707]
+ // /repo/b [755]
+ // /repo/b/c [777]
+ // /repo/b/d [755]
+ // /repo/b/d/e [755]
+ // /repo/b/d/f [755]
+ // /repo/b/d/f/g [755]
+ // /repo/b/d/f/g/h [755]
// /secret [700]
basePath, err := os.MkdirTemp("", "")
@@ -28,12 +28,12 @@ func ExampleDirStructure() {
return
}
- ds := NewDirStructure(basePath, 0o0755)
- secret := ds.ChildDir("secret", 0o0700)
- repo := ds.ChildDir("repo", 0o0777)
- _ = repo.ChildDir("a", 0o0700)
- b := repo.ChildDir("b", 0o0707)
- c := b.ChildDir("c", 0o0750)
+ ds := NewDirStructure(basePath, PublicReadPermission)
+ secret := ds.ChildDir("secret", AdminOnlyPermission)
+ repo := ds.ChildDir("repo", PublicWritePermission)
+ _ = repo.ChildDir("a", AdminOnlyPermission)
+ b := repo.ChildDir("b", PublicReadPermission)
+ c := b.ChildDir("c", PublicWritePermission)
err = ds.Ensure()
if err != nil {
diff --git a/cmds/notifier/main.go b/cmds/notifier/main.go
index e40487bb..a11bea2a 100644
--- a/cmds/notifier/main.go
+++ b/cmds/notifier/main.go
@@ -225,14 +225,14 @@ func configureRegistry(mustLoadIndex bool) error {
// Remove left over quotes.
dataDir = strings.Trim(dataDir, `\"`)
// Initialize data root.
- err := dataroot.Initialize(dataDir, 0o0755)
+ err := dataroot.Initialize(dataDir, utils.PublicReadPermission)
if err != nil {
return fmt.Errorf("failed to initialize data root: %w", err)
}
dataRoot = dataroot.Root()
// Initialize registry.
- err = registry.Initialize(dataRoot.ChildDir("updates", 0o0755))
+ err = registry.Initialize(dataRoot.ChildDir("updates", utils.PublicReadPermission))
if err != nil {
return err
}
diff --git a/cmds/updatemgr/main.go b/cmds/updatemgr/main.go
index acd9a0d4..4a999f49 100644
--- a/cmds/updatemgr/main.go
+++ b/cmds/updatemgr/main.go
@@ -31,7 +31,7 @@ var rootCmd = &cobra.Command{
}
registry = &updater.ResourceRegistry{}
- err = registry.Initialize(utils.NewDirStructure(absDistPath, 0o0755))
+ err = registry.Initialize(utils.NewDirStructure(absDistPath, utils.PublicReadPermission))
if err != nil {
return err
}
diff --git a/cmds/updatemgr/release.go b/cmds/updatemgr/release.go
index 0f5d596e..e6642a63 100644
--- a/cmds/updatemgr/release.go
+++ b/cmds/updatemgr/release.go
@@ -11,6 +11,7 @@ import (
"github.com/spf13/cobra"
"github.com/safing/portmaster/base/updater"
+ "github.com/safing/portmaster/base/utils"
)
var (
@@ -63,7 +64,7 @@ func release(cmd *cobra.Command, args []string) error {
fmt.Println("aborted...")
return nil
}
- symlinksDir := registry.StorageDir().ChildDir("latest", 0o755)
+ symlinksDir := registry.StorageDir().ChildDir("latest", utils.PublicReadPermission)
err = registry.CreateSymlinks(symlinksDir)
if err != nil {
return err
diff --git a/service/updates/upgrader.go b/service/updates/upgrader.go
index f1d3c297..b07d649e 100644
--- a/service/updates/upgrader.go
+++ b/service/updates/upgrader.go
@@ -351,7 +351,7 @@ func upgradeBinary(fileToUpgrade string, file *updater.File) error {
// check permissions
if onWindows {
- utils.SetExecPermission(fileToUpgrade, utils.PublicReadPermission)
+ _ = utils.SetExecPermission(fileToUpgrade, utils.PublicReadPermission)
} else {
perm := utils.PublicReadPermission
info, err := os.Stat(fileToUpgrade)
From 9d874daed2ff360e016821fc4da0a09ae5eef536 Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Fri, 6 Dec 2024 14:34:54 +0100
Subject: [PATCH 3/4] Simplify windows acl calls and switch to using SIDs
---
base/utils/permissions_windows.go | 63 +++++++++++++++++++++++++------
1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/base/utils/permissions_windows.go b/base/utils/permissions_windows.go
index 5f36ebb9..47aa0c26 100644
--- a/base/utils/permissions_windows.go
+++ b/base/utils/permissions_windows.go
@@ -7,32 +7,71 @@ import (
"golang.org/x/sys/windows"
)
+var (
+ systemSID *windows.SID
+ adminsSID *windows.SID
+ usersSID *windows.SID
+)
+
+func init() {
+ var err error
+ systemSID, err = windows.StringToSid("S-1-5") // NT Authority / SYSTEM
+ if err != nil {
+ panic(err)
+ }
+ adminsSID, err = windows.StringToSid("S-1-5-32-544") // Administrators
+ if err != nil {
+ panic(err)
+ }
+ usersSID, err = windows.StringToSid("S-1-5-32-545") // Users
+ if err != nil {
+ panic(err)
+ }
+}
+
+// SetDirPermission sets the permission of a directory.
func SetDirPermission(path string, perm FSPermission) error {
- setWindowsFilePermissions(path, perm)
+ SetFilePermission(path, perm)
return nil
}
// SetExecPermission sets the permission of an executable file.
func SetExecPermission(path string, perm FSPermission) error {
- return SetDirPermission(path, perm)
+ SetFilePermission(path, perm)
+ return nil
}
-func setWindowsFilePermissions(path string, perm FSPermission) {
+// SetFilePermission sets the permission of a non executable file.
+func SetFilePermission(path string, perm FSPermission) {
switch perm {
case AdminOnlyPermission:
// Set only admin rights, remove all others.
- acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
+ acl.Apply(
+ path,
+ true,
+ false,
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, systemSID),
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, adminsSID),
+ )
case PublicReadPermission:
// Set admin rights and read/execute rights for users, remove all others.
- acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
- acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_EXECUTE, "Users"))
- acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_READ, "Users"))
+ acl.Apply(
+ path,
+ true,
+ false,
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, systemSID),
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, adminsSID),
+ acl.GrantSid(windows.GENERIC_READ|windows.GENERIC_EXECUTE, usersSID),
+ )
case PublicWritePermission:
// Set full control to admin and regular users. Guest users will not have access.
- acl.Apply(path, true, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Administrators"))
- acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "Users"))
+ acl.Apply(
+ path,
+ true,
+ false,
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, systemSID),
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, adminsSID),
+ acl.GrantSid(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, usersSID),
+ )
}
-
- // For completeness
- acl.Apply(path, false, false, acl.GrantName(windows.GENERIC_ALL|windows.STANDARD_RIGHTS_ALL, "SYSTEM"))
}
From 475d69f8a241f52e3c082a4a6f98a3fcabf89d19 Mon Sep 17 00:00:00 2001
From: Vladimir Stoilov <vladimir@safing.io>
Date: Fri, 6 Dec 2024 16:45:37 +0200
Subject: [PATCH 4/4] [service] Fix windows system SID
---
base/utils/permissions_windows.go | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/base/utils/permissions_windows.go b/base/utils/permissions_windows.go
index 47aa0c26..13cbf583 100644
--- a/base/utils/permissions_windows.go
+++ b/base/utils/permissions_windows.go
@@ -14,8 +14,10 @@ var (
)
func init() {
+ // Initialize Security ID for all need groups.
+ // Reference: https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-identifiers
var err error
- systemSID, err = windows.StringToSid("S-1-5") // NT Authority / SYSTEM
+ systemSID, err = windows.StringToSid("S-1-5-18") // SYSTEM (Local System)
if err != nil {
panic(err)
}