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
safing · Nov 26, 2024 · 943b9b7 · 943b9b7
1 parent ca88c6d
commit 943b9b7
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)