From 22253c4e9e53918d9675dff5ffed67b3baa2304f Mon Sep 17 00:00:00 2001 From: Vladimir Stoilov 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 0ed0b7e6a..c737dc2f7 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 f84a01086..9c9aa1edc 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 7965439ab..0b4f175c9 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 296b342f3..758052559 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 150037ed5..f7f373dc0 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 8deda74ed..4450fe98b 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 bb59960fc..6ede989d7 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 000000000..d76907242 --- /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 000000000..ac48c21a5 --- /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 5a50d97e0..e33a920fd 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 e327963f0..a95c500b1 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 f764dfbff..d13a4bd68 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 3b1cc82f5..fa67048fd 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 a1cd6aea9..912fe3cc5 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 911ef99c2..652fcd527 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 630808e50..b9f7220fb 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 bb942993a..c5a41d43e 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 4963bced5..f1d3c297f 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 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 0b4f175c9..4b04f41d3 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 4450fe98b..a2bf5fcde 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 a8978f68c..1b409b257 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 6ede989d7..5eb456b13 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 ac48c21a5..5f36ebb9b 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 e33a920fd..8e1f0786a 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 2acfebd21..d3debcf72 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 e40487bbc..a11bea2ac 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 acd9a0d47..4a999f49f 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 0f5d596e7..e6642a638 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 f1d3c297f..b07d649e9 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 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 5f36ebb9b..47aa0c264 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 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 47aa0c264..13cbf5839 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) }