From 943b9b78591baa6c0b33f0453474030e2335d65c Mon Sep 17 00:00:00 2001 From: Vladimir Stoilov Date: Tue, 26 Nov 2024 17:00:01 +0200 Subject: [PATCH] Fix file permissions on windows (#1758) * [service] Set file permissions on windows * [service] Fix minor windows permission bugs * [service] Fix permission bugs * [service] Fix windows non admin user start --- base/database/storage/fstree/fstree.go | 12 ++++++++---- base/updater/fetch.go | 8 +++++++- base/utils/fs.go | 15 ++++++++++++--- base/utils/renameio/writefile.go | 14 ++++++++++++-- cmds/portmaster-start/run.go | 2 +- go.mod | 1 + go.sum | 3 +++ service/updates/helper/electron.go | 2 +- service/updates/upgrader.go | 8 +++++++- 9 files changed, 52 insertions(+), 13 deletions(-) diff --git a/base/database/storage/fstree/fstree.go b/base/database/storage/fstree/fstree.go index 44cf384f2..7965439ab 100644 --- a/base/database/storage/fstree/fstree.go +++ b/base/database/storage/fstree/fstree.go @@ -15,6 +15,7 @@ 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" @@ -288,10 +289,13 @@ 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 { - if err := t.Chmod(perm); err != nil { - return err - } + if onWindows { + err = acl.Chmod(filename, perm) + } else { + err = t.Chmod(perm) + } + if err != nil { + return err } if _, err := t.Write(data); err != nil { diff --git a/base/updater/fetch.go b/base/updater/fetch.go index f324709d4..150037ed5 100644 --- a/base/updater/fetch.go +++ b/base/updater/fetch.go @@ -14,6 +14,7 @@ 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" @@ -136,7 +137,12 @@ 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 { + 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 { diff --git a/base/utils/fs.go b/base/utils/fs.go index b612a0699..bb59960fc 100644 --- a/base/utils/fs.go +++ b/base/utils/fs.go @@ -6,6 +6,8 @@ import ( "io/fs" "os" "runtime" + + "github.com/hectane/go-acl" ) const isWindows = runtime.GOOS == "windows" @@ -20,8 +22,9 @@ func EnsureDirectory(path string, perm os.FileMode) error { if f.IsDir() { // directory exists, check permissions if isWindows { - // TODO: set correct permission on windows - // acl.Chmod(path, perm) + // Ignore windows permission error. For none admin users it will always fail. + acl.Chmod(path, perm) + return nil } else if f.Mode().Perm() != perm { return os.Chmod(path, perm) } @@ -38,7 +41,13 @@ func EnsureDirectory(path string, perm os.FileMode) error { if err != nil { return fmt.Errorf("could not create dir %s: %w", path, err) } - return os.Chmod(path, perm) + 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) + } } // other error opening path return fmt.Errorf("failed to access %s: %w", path, err) diff --git a/base/utils/renameio/writefile.go b/base/utils/renameio/writefile.go index 211530255..073a1e1b0 100644 --- a/base/utils/renameio/writefile.go +++ b/base/utils/renameio/writefile.go @@ -1,6 +1,11 @@ package renameio -import "os" +import ( + "os" + "runtime" + + "github.com/hectane/go-acl" +) // WriteFile mirrors os.WriteFile, replacing an existing file with the same // name atomically. @@ -14,7 +19,12 @@ func WriteFile(filename string, data []byte, perm os.FileMode) error { }() // Set permissions before writing data, in case the data is sensitive. - if err := t.Chmod(perm); err != nil { + if runtime.GOOS == "windows" { + err = acl.Chmod(t.path, perm) + } else { + err = t.Chmod(perm) + } + if err != nil { return err } diff --git a/cmds/portmaster-start/run.go b/cmds/portmaster-start/run.go index f807fd69d..48ad36cbe 100644 --- a/cmds/portmaster-start/run.go +++ b/cmds/portmaster-start/run.go @@ -272,7 +272,7 @@ func fixExecPerm(path string) error { return nil } - if err := os.Chmod(path, 0o0755); err != nil { //nolint:gosec // Set execution rights. + if err := os.Chmod(path, 0o0755); err != nil { //nolint:gosec return fmt.Errorf("failed to chmod %s: %w", path, err) } diff --git a/go.mod b/go.mod index 80685cce6..d40c14101 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,7 @@ require ( github.com/gorilla/websocket v1.5.3 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-version v1.7.0 + github.com/hectane/go-acl v0.0.0-20230122075934-ca0b05cb1adb github.com/jackc/puddle/v2 v2.2.2 github.com/lmittmann/tint v1.0.5 github.com/maruel/panicparse/v2 v2.3.1 diff --git a/go.sum b/go.sum index 4a0cac3fc..c2bfeae66 100644 --- a/go.sum +++ b/go.sum @@ -137,6 +137,8 @@ github.com/hashicorp/go-version v1.7.0 h1:5tqGy27NaOTB8yJKUZELlFAS/LTKJkrmONwQKe github.com/hashicorp/go-version v1.7.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/hcl v0.0.0-20170914154624-68e816d1c783/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= +github.com/hectane/go-acl v0.0.0-20230122075934-ca0b05cb1adb h1:PGufWXXDq9yaev6xX1YQauaO1MV90e6Mpoq1I7Lz/VM= +github.com/hectane/go-acl v0.0.0-20230122075934-ca0b05cb1adb/go.mod h1:QiyDdbZLaJ/mZP4Zwc9g2QsfaEA4o7XvvgZegSci5/E= github.com/inconshreveable/log15 v0.0.0-20170622235902-74a0988b5f80/go.mod h1:cOaXtrgN4ScfRrD9Bre7U1thNq5RtJ8ZoP4iXVGRj6o= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -388,6 +390,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190411185658-b44545bcd369/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20190529164535-6a60838ec259/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/service/updates/helper/electron.go b/service/updates/helper/electron.go index 4c8c4a07d..2fa6810be 100644 --- a/service/updates/helper/electron.go +++ b/service/updates/helper/electron.go @@ -48,9 +48,9 @@ func EnsureChromeSandboxPermissions(reg *updater.ResourceRegistry) error { sandboxFile := filepath.Join(unpackedPath, "chrome-sandbox") if err := os.Chmod(sandboxFile, 0o0755|os.ModeSetuid); err != nil { log.Errorf(suidBitWarning, 0o0755|os.ModeSetuid, sandboxFile) - return fmt.Errorf("failed to chmod: %w", err) } + log.Debugf("updates: fixed SUID permission for chrome-sandbox") return nil diff --git a/service/updates/upgrader.go b/service/updates/upgrader.go index 622b3909b..4963bced5 100644 --- a/service/updates/upgrader.go +++ b/service/updates/upgrader.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/hectane/go-acl" processInfo "github.com/shirou/gopsutil/process" "github.com/tevino/abool" @@ -349,7 +350,12 @@ func upgradeBinary(fileToUpgrade string, file *updater.File) error { } // check permissions - if !onWindows { + if onWindows { + err = acl.Chmod(fileToUpgrade, 0o0755) + if err != nil { + return fmt.Errorf("failed to set permissions on %s: %w", fileToUpgrade, err) + } + } else { info, err := os.Stat(fileToUpgrade) if err != nil { return fmt.Errorf("failed to get file info on %s: %w", fileToUpgrade, err)