all: revert permcheck

internal/aghos/permission_windows.go

@@ -64,7 +64,7 @@ func stat(name string) (fi os.FileInfo, err error) {
 		case entrySid.Equals(group):
 			groupMask |= ace.Mask
 		default:
-			otherMask = ace.Mask
+			otherMask |= ace.Mask
 		}
 	}
 
@@ -143,7 +143,7 @@ func chmod(name string, perm fs.FileMode) (err error) {
 			continue
 		}
 
-		var trustee *windows.TRUSTEE
+		var trustee windows.TRUSTEE
 		trustee, err = newWellKnownTrustee(sidMask.Key)
 		if err != nil {
 			errs = append(errs, err)
@@ -155,7 +155,7 @@ func chmod(name string, perm fs.FileMode) (err error) {
 			AccessPermissions: sidMask.Value,
 			AccessMode:        windows.GRANT_ACCESS,
 			Inheritance:       windows.NO_INHERITANCE,
-			Trustee:           *trustee,
+			Trustee:           trustee,
 		})
 	}
 
@@ -208,9 +208,11 @@ func mkdir(name string, perm os.FileMode) (err error) {
 func mkdirAll(path string, perm os.FileMode) (err error) {
 	parent, _ := filepath.Split(path)
 
-	err = os.MkdirAll(parent, perm)
-	if err != nil && !errors.Is(err, os.ErrExist) {
-		return fmt.Errorf("creating parent directories: %w", err)
+	if parent != "" {
+		err = os.MkdirAll(parent, perm)
+		if err != nil && !errors.Is(err, os.ErrExist) {
+			return fmt.Errorf("creating parent directories: %w", err)
+		}
 	}
 
 	err = mkdir(path, perm)
@@ -258,13 +260,13 @@ func openFile(name string, flag int, perm os.FileMode) (file *os.File, err error
 }
 
 // newWellKnownTrustee returns a trustee for a well-known SID.
-func newWellKnownTrustee(stype windows.WELL_KNOWN_SID_TYPE) (t *windows.TRUSTEE, err error) {
+func newWellKnownTrustee(stype windows.WELL_KNOWN_SID_TYPE) (t windows.TRUSTEE, err error) {
 	sid, err := windows.CreateWellKnownSid(stype)
 	if err != nil {
-		return nil, fmt.Errorf("creating sid for type %d: %w", stype, err)
+		return windows.TRUSTEE{}, fmt.Errorf("creating sid for type %d: %w", stype, err)
 	}
 
-	return &windows.TRUSTEE{
+	return windows.TRUSTEE{
 		TrusteeForm:  windows.TRUSTEE_IS_SID,
 		TrusteeValue: windows.TrusteeValueFromSID(sid),
 	}, nil
@@ -280,14 +282,14 @@ const (
 // Windows access masks for appropriate UNIX file mode permission bits and
 // file types.
 const (
-	fileReadRights = windows.READ_CONTROL |
+	fileReadRights windows.ACCESS_MASK = windows.READ_CONTROL |
 		windows.FILE_READ_DATA |
 		windows.FILE_READ_ATTRIBUTES |
 		windows.FILE_READ_EA |
 		windows.SYNCHRONIZE |
 		windows.ACCESS_SYSTEM_SECURITY
 
-	fileWriteRights = windows.WRITE_DAC |
+	fileWriteRights windows.ACCESS_MASK = windows.WRITE_DAC |
 		windows.WRITE_OWNER |
 		windows.FILE_WRITE_DATA |
 		windows.FILE_WRITE_ATTRIBUTES |
@@ -297,16 +299,16 @@ const (
 		windows.SYNCHRONIZE |
 		windows.ACCESS_SYSTEM_SECURITY
 
-	fileExecuteRights = windows.FILE_EXECUTE
+	fileExecuteRights windows.ACCESS_MASK = windows.FILE_EXECUTE
 
-	dirReadRights = windows.READ_CONTROL |
+	dirReadRights windows.ACCESS_MASK = windows.READ_CONTROL |
 		windows.FILE_LIST_DIRECTORY |
 		windows.FILE_READ_EA |
 		windows.FILE_READ_ATTRIBUTES<<1 |
 		windows.SYNCHRONIZE |
 		windows.ACCESS_SYSTEM_SECURITY
 
-	dirWriteRights = windows.WRITE_DAC |
+	dirWriteRights windows.ACCESS_MASK = windows.WRITE_DAC |
 		windows.WRITE_OWNER |
 		windows.DELETE |
 		windows.FILE_WRITE_DATA |
@@ -316,7 +318,7 @@ const (
 		windows.SYNCHRONIZE |
 		windows.ACCESS_SYSTEM_SECURITY
 
-	dirExecuteRights = windows.FILE_TRAVERSE
+	dirExecuteRights windows.ACCESS_MASK = windows.FILE_TRAVERSE
 )
 
 // permToMasks converts a UNIX file mode permissions to the corresponding
@@ -336,26 +338,19 @@ func permToMasks(fm os.FileMode, isDir bool) (owner, group, world windows.ACCESS
 // corresponding Windows access mask.  The [isDir] argument is used to set
 // specific access bits for directories.
 func permToMask(p byte, isDir bool) (mask windows.ACCESS_MASK) {
+	readRights, writeRights, executeRights := fileReadRights, fileWriteRights, fileExecuteRights
+	if isDir {
+		readRights, writeRights, executeRights = dirReadRights, dirWriteRights, dirExecuteRights
+	}
+
 	if p&permRead != 0 {
-		if isDir {
-			mask |= dirReadRights
-		} else {
-			mask |= fileReadRights
-		}
+		mask |= readRights
 	}
 	if p&permWrite != 0 {
-		if isDir {
-			mask |= dirWriteRights
-		} else {
-			mask |= fileWriteRights
-		}
+		mask |= writeRights
 	}
 	if p&permExecute != 0 {
-		if isDir {
-			mask |= dirExecuteRights
-		} else {
-			mask |= fileExecuteRights
-		}
+		mask |= executeRights
 	}
 
 	return mask
@@ -374,26 +369,23 @@ func masksToPerm(u, g, o windows.ACCESS_MASK, isDir bool) (perm fs.FileMode) {
 // maskToPerm converts a Windows access mask to the corresponding UNIX file
 // mode permission bits.
 func maskToPerm(mask windows.ACCESS_MASK, isDir bool) (perm byte) {
+	readMask, writeMask, executeMask := fileReadRights, fileWriteRights, fileExecuteRights
 	if isDir {
-		if mask&dirReadRights != 0 {
-			perm |= permRead
-		}
-		if mask&dirWriteRights != 0 {
-			perm |= permWrite
-		}
-		if mask&dirExecuteRights != 0 {
-			perm |= permExecute
-		}
-	} else {
-		if mask&fileReadRights != 0 {
-			perm |= permRead
-		}
-		if mask&fileWriteRights != 0 {
-			perm |= permWrite
-		}
-		if mask&fileExecuteRights != 0 {
-			perm |= permExecute
-		}
+		readMask, writeMask, executeMask = dirReadRights, dirWriteRights, dirExecuteRights
+	}
+
+	// Remove common bits to avoid false positive detection of unset rights.
+	readMask ^= windows.SYNCHRONIZE | windows.ACCESS_SYSTEM_SECURITY
+	writeMask ^= windows.SYNCHRONIZE | windows.ACCESS_SYSTEM_SECURITY
+
+	if mask&readMask != 0 {
+		perm |= permRead
+	}
+	if mask&writeMask != 0 {
+		perm |= permWrite
+	}
+	if mask&executeMask != 0 {
+		perm |= permExecute
 	}
 
 	return perm

internal/aghos/permission_windows_internal_test.go

@@ -29,14 +29,14 @@ func TestPermToMasks(t *testing.T) {
 		isDir:     false,
 	}, {
 		name:      "user_write",
-		perm:      0o010_000_000,
+		perm:      0b010_000_000,
 		wantUser:  fileWriteRights,
 		wantGroup: 0,
 		wantOther: 0,
 		isDir:     false,
 	}, {
 		name:      "group_read",
-		perm:      0o000_010_000,
+		perm:      0b000_100_000,
 		wantUser:  0,
 		wantGroup: fileReadRights,
 		wantOther: 0,
@@ -50,7 +50,7 @@ func TestPermToMasks(t *testing.T) {
 		isDir:     true,
 	}, {
 		name:      "user_write_dir",
-		perm:      0o010_000_000,
+		perm:      0b010_000_000,
 		wantUser:  dirWriteRights,
 		wantGroup: 0,
 		wantOther: 0,
@@ -91,14 +91,14 @@ func TestMasksToPerm(t *testing.T) {
 		user:     fileWriteRights,
 		group:    0,
 		other:    0,
-		wantPerm: 0o010_000_000,
+		wantPerm: 0b010_000_000,
 		isDir:    false,
 	}, {
 		name:     "group_read",
 		user:     0,
 		group:    fileReadRights,
 		other:    0,
-		wantPerm: 0o000_010_000,
+		wantPerm: 0b000_100_000,
 		isDir:    false,
 	}, {
 		name:     "no_access",
@@ -119,7 +119,7 @@ func TestMasksToPerm(t *testing.T) {
 		user:     dirWriteRights,
 		group:    0,
 		other:    0,
-		wantPerm: 0o010_000_000,
+		wantPerm: 0b010_000_000,
 		isDir:    true,
 	}}
 

internal/home/home.go

@@ -687,7 +687,7 @@ func run(opts options, clientBuildFS fs.FS, done chan struct{}) {
 	}
 
 	if permcheck.NeedsMigration(confPath) {
-		permcheck.Migrate(Context.workDir, dataDir, statsDir, querylogDir, confPath, execPath)
+		permcheck.Migrate(Context.workDir, dataDir, statsDir, querylogDir, confPath)
 	}
 
 	permcheck.Check(Context.workDir, dataDir, statsDir, querylogDir, confPath)

internal/permcheck/migrate.go

@@ -32,14 +32,11 @@ func NeedsMigration(confFilePath string) (ok bool) {
 
 // Migrate attempts to change the permissions of AdGuard Home's files.  It logs
 // the results at an appropriate level.
-func Migrate(workDir, dataDir, statsDir, querylogDir, confFilePath, execPath string) {
+func Migrate(workDir, dataDir, statsDir, querylogDir, confFilePath string) {
 	chmodDir(workDir)
 
 	chmodFile(confFilePath)
 
-	// Allow the binary to be executed.
-	chmodPath(execPath, typeFile, aghos.DefaultPermFile|0b001_000_000)
-
 	// TODO(a.garipov): Put all paths in one place and remove this duplication.
 	chmodDir(dataDir)
 	chmodDir(filepath.Join(dataDir, "filters"))