permcheck: imp code

internal/permcheck/check_unix.go

@@ -4,15 +4,9 @@ package permcheck
 
 import (
 	"context"
-	"fmt"
-	"io/fs"
 	"log/slog"
-	"os"
-	"path/filepath"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
-	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/logutil/slogutil"
 )
 
 // check is the Unix-specific implementation of [Check].
@@ -27,26 +21,13 @@ func check(
 ) {
 	dirLoggger, fileLogger := l.With("type", typeDir), l.With("type", typeFile)
 
-	checkDir(ctx, dirLoggger, workDir)
+	for _, ent := range entities(workDir, dataDir, statsDir, querylogDir, confFilePath) {
-
+		if ent.Value {
-	checkFile(ctx, fileLogger, confFilePath)
+			checkDir(ctx, dirLoggger, ent.Key)
-
+		} else {
-	// TODO(a.garipov): Put all paths in one place and remove this duplication.
+			checkFile(ctx, fileLogger, ent.Key)
-	checkDir(ctx, dirLoggger, dataDir)
-	checkDir(ctx, dirLoggger, filepath.Join(dataDir, "filters"))
-	checkFile(ctx, fileLogger, filepath.Join(dataDir, "sessions.db"))
-	checkFile(ctx, fileLogger, filepath.Join(dataDir, "leases.json"))
-
-	if dataDir != querylogDir {
-		checkDir(ctx, dirLoggger, querylogDir)
 		}
-	checkFile(ctx, fileLogger, filepath.Join(querylogDir, "querylog.json"))
-	checkFile(ctx, fileLogger, filepath.Join(querylogDir, "querylog.json.1"))
-
-	if dataDir != statsDir {
-		checkDir(ctx, dirLoggger, statsDir)
 	}
-	checkFile(ctx, fileLogger, filepath.Join(statsDir, "stats.db"))
 }
 
 // checkDir checks the permissions of a single directory.  The results are
@@ -60,29 +41,3 @@ func checkDir(ctx context.Context, l *slog.Logger, dirPath string) {
 func checkFile(ctx context.Context, l *slog.Logger, filePath string) {
 	checkPath(ctx, l, filePath, aghos.DefaultPermFile)
 }
-
-// checkPath checks the permissions of a single filesystem entity.  The results
-// are logged at the appropriate level.
-func checkPath(ctx context.Context, l *slog.Logger, fpath string, want fs.FileMode) {
-	l = l.With("path", fpath)
-	s, err := os.Stat(fpath)
-	if err != nil {
-		lvl := slog.LevelError
-		if errors.Is(err, os.ErrNotExist) {
-			lvl = slog.LevelDebug
-		}
-
-		l.Log(ctx, lvl, "checking permissions", slogutil.KeyError, err)
-
-		return
-	}
-
-	// TODO(a.garipov): Add a more fine-grained check and result reporting.
-	perm := s.Mode().Perm()
-	if perm == want {
-		return
-	}
-
-	permOct, wantOct := fmt.Sprintf("%#o", perm), fmt.Sprintf("%#o", want)
-	l.WarnContext(ctx, "found unexpected permissions", "perm", permOct, "want", wantOct)
-}

internal/permcheck/migrate_unix.go

@@ -4,11 +4,8 @@ package permcheck
 
 import (
 	"context"
-	"fmt"
-	"io/fs"
 	"log/slog"
 	"os"
-	"path/filepath"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghos"
 	"github.com/AdguardTeam/golibs/errors"
@@ -47,26 +44,13 @@ func migrate(
 ) {
 	dirLoggger, fileLogger := l.With("type", typeDir), l.With("type", typeFile)
 
-	chmodDir(ctx, dirLoggger, workDir)
+	for _, ent := range entities(workDir, dataDir, statsDir, querylogDir, confFilePath) {
-
+		if ent.Value {
-	chmodFile(ctx, fileLogger, confFilePath)
+			chmodDir(ctx, dirLoggger, ent.Key)
-
+		} else {
-	// TODO(a.garipov): Put all paths in one place and remove this duplication.
+			chmodFile(ctx, fileLogger, ent.Key)
-	chmodDir(ctx, dirLoggger, dataDir)
-	chmodDir(ctx, dirLoggger, filepath.Join(dataDir, "filters"))
-	chmodFile(ctx, fileLogger, filepath.Join(dataDir, "sessions.db"))
-	chmodFile(ctx, fileLogger, filepath.Join(dataDir, "leases.json"))
-
-	if dataDir != querylogDir {
-		chmodDir(ctx, dirLoggger, querylogDir)
 		}
-	chmodFile(ctx, fileLogger, filepath.Join(querylogDir, "querylog.json"))
-	chmodFile(ctx, fileLogger, filepath.Join(querylogDir, "querylog.json.1"))
-
-	if dataDir != statsDir {
-		chmodDir(ctx, dirLoggger, statsDir)
 	}
-	chmodFile(ctx, fileLogger, filepath.Join(statsDir, "stats.db"))
 }
 
 // chmodDir changes the permissions of a single directory.  The results are
@@ -80,28 +64,3 @@ func chmodDir(ctx context.Context, l *slog.Logger, dirPath string) {
 func chmodFile(ctx context.Context, l *slog.Logger, filePath string) {
 	chmodPath(ctx, l, filePath, aghos.DefaultPermFile)
 }
-
-// chmodPath changes the permissions of a single filesystem entity.  The results
-// are logged at the appropriate level.
-func chmodPath(ctx context.Context, l *slog.Logger, fpath string, fm fs.FileMode) {
-	var lvl slog.Level
-	var msg string
-	args := []any{"path", fpath}
-
-	switch err := os.Chmod(fpath, fm); {
-	case err == nil:
-		lvl = slog.LevelInfo
-		msg = "changed permissions"
-	case errors.Is(err, os.ErrNotExist):
-		lvl = slog.LevelDebug
-		msg = "checking permissions"
-		args = append(args, slogutil.KeyError, err)
-	default:
-		lvl = slog.LevelError
-		msg = "cannot change permissions; this can leave your system vulnerable, see " +
-			"https://adguard-dns.io/kb/adguard-home/running-securely/#os-service-concerns"
-		args = append(args, "target_perm", fmt.Sprintf("%#o", fm), slogutil.KeyError, err)
-	}
-
-	l.Log(ctx, lvl, msg, args...)
-}

internal/permcheck/migrate_windows.go

@@ -57,7 +57,10 @@ func needsMigration(ctx context.Context, l *slog.Logger, workDir, _ string) (ok
 
 // migrate is the Windows-specific implementation of [Migrate].
 //
-// It
+// It sets the owner to administrators and adds a full control access control
+// entry for the account.  It also removes all non-administrator access control
+// entries, and keeps deny access control entries.  For any created or modified
+// entry it sets the propagation flags to be inherited by child objects.
 func migrate(ctx context.Context, logger *slog.Logger, workDir, _, _, _, _ string) {
 	l := logger.With("type", typeDir, "path", workDir)
 
@@ -80,7 +83,7 @@ func migrate(ctx context.Context, logger *slog.Logger, workDir, _, _, _, _ strin
 
 	// TODO(e.burkov):  Check for duplicates?
 	var accessEntries []windows.EXPLICIT_ACCESS
-	var useACL bool
+	var setACL bool
 	// Iterate over the access control entries in DACL to determine if its
 	// migration is needed.
 	err = rangeACEs(dacl, func(
@@ -94,18 +97,18 @@ func migrate(ctx context.Context, logger *slog.Logger, workDir, _, _, _, _ strin
 			// the access restrictions, which shouldn't be lost.
 			l.InfoContext(ctx, "migrating deny access control entry", "sid", sid)
 			accessEntries = append(accessEntries, newDenyExplicitAccess(sid, mask))
-			useACL = true
+			setACL = true
 		case !sid.IsWellKnown(windows.WinBuiltinAdministratorsSid):
 			// Remove non-administrator ACEs, since such accounts should not
 			// have any access rights.
 			l.InfoContext(ctx, "removing access control entry", "sid", sid)
-			useACL = true
+			setACL = true
 		default:
 			// Administrators should have full control.  Don't add a new entry
 			// here since it will be added later in case there are other
 			// required entries.
 			l.InfoContext(ctx, "migrating access control entry", "sid", sid, "mask", mask)
-			useACL = useACL || mask&fullControlMask != fullControlMask
+			setACL = setACL || mask&fullControlMask != fullControlMask
 		}
 
 		return true
@@ -116,7 +119,7 @@ func migrate(ctx context.Context, logger *slog.Logger, workDir, _, _, _, _ strin
 		return
 	}
 
-	if useACL {
+	if setACL {
 		accessEntries = append(accessEntries, newFullExplicitAccess(owner))
 	}
 

internal/permcheck/security_unix.go

@@ -0,0 +1,121 @@
+//go:build unix
+
+package permcheck
+
+import (
+	"context"
+	"fmt"
+	"io/fs"
+	"log/slog"
+	"os"
+	"path/filepath"
+
+	"github.com/AdguardTeam/golibs/container"
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/logutil/slogutil"
+)
+
+// entity is a filesystem entity with a path and a flag indicating whether it is
+// a directory.
+type entity = container.KeyValue[string, bool]
+
+// entities returns a list of filesystem entities that need to be ranged over.
+func entities(workDir, dataDir, statsDir, querylogDir, confFilePath string) (ents []entity) {
+	ents = container.KeyValues[string, bool]{{
+		Key:   workDir,
+		Value: true,
+	}, {
+		Key:   confFilePath,
+		Value: false,
+	}, {
+		Key:   dataDir,
+		Value: true,
+	}, {
+		Key:   filepath.Join(dataDir, "filters"),
+		Value: true,
+	}, {
+		Key:   filepath.Join(dataDir, "sessions.db"),
+		Value: false,
+	}, {
+		Key:   filepath.Join(dataDir, "leases.json"),
+		Value: false,
+	}}
+
+	if dataDir != querylogDir {
+		ents = append(ents, entity{
+			Key:   querylogDir,
+			Value: true,
+		})
+	}
+	ents = append(ents, []entity{{
+		Key:   filepath.Join(querylogDir, "querylog.json"),
+		Value: false,
+	}, {
+		Key:   filepath.Join(querylogDir, "querylog.json.1"),
+		Value: false,
+	}}...)
+
+	if dataDir != statsDir {
+		ents = append(ents, entity{
+			Key:   statsDir,
+			Value: true,
+		})
+	}
+	ents = append(ents, entity{
+		Key: filepath.Join(statsDir, "stats.db"),
+	})
+
+	return ents
+}
+
+// checkPath checks the permissions of a single filesystem entity.  The results
+// are logged at the appropriate level.
+func checkPath(ctx context.Context, l *slog.Logger, entPath string, want fs.FileMode) {
+	l = l.With("path", entPath)
+
+	s, err := os.Stat(entPath)
+	if err != nil {
+		lvl := slog.LevelError
+		if errors.Is(err, os.ErrNotExist) {
+			lvl = slog.LevelDebug
+		}
+
+		l.Log(ctx, lvl, "checking permissions", slogutil.KeyError, err)
+
+		return
+	}
+
+	// TODO(a.garipov): Add a more fine-grained check and result reporting.
+	perm := s.Mode().Perm()
+	if perm == want {
+		return
+	}
+
+	permOct, wantOct := fmt.Sprintf("%#o", perm), fmt.Sprintf("%#o", want)
+	l.WarnContext(ctx, "found unexpected permissions", "perm", permOct, "want", wantOct)
+}
+
+// chmodPath changes the permissions of a single filesystem entity.  The results
+// are logged at the appropriate level.
+func chmodPath(ctx context.Context, l *slog.Logger, entPath string, fm fs.FileMode) {
+	var lvl slog.Level
+	var msg string
+	args := []any{"path", entPath}
+
+	switch err := os.Chmod(entPath, fm); {
+	case err == nil:
+		lvl = slog.LevelInfo
+		msg = "changed permissions"
+	case errors.Is(err, os.ErrNotExist):
+		lvl = slog.LevelDebug
+		msg = "checking permissions"
+		args = append(args, slogutil.KeyError, err)
+	default:
+		lvl = slog.LevelError
+		msg = "cannot change permissions; this can leave your system vulnerable, see " +
+			"https://adguard-dns.io/kb/adguard-home/running-securely/#os-service-concerns"
+		args = append(args, "target_perm", fmt.Sprintf("%#o", fm), slogutil.KeyError, err)
+	}
+
+	l.Log(ctx, lvl, msg, args...)
+}