diff --git a/agh_test.exe b/agh_test.exe new file mode 100755 index 00000000..e4549a56 Binary files /dev/null and b/agh_test.exe differ diff --git a/internal/aghos/permission_windows.go b/internal/aghos/permission_windows.go index 64bcd757..cc5a4aa8 100644 --- a/internal/aghos/permission_windows.go +++ b/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 diff --git a/internal/aghos/permission_windows_internal_test.go b/internal/aghos/permission_windows_internal_test.go index ed3566a4..3837fda1 100644 --- a/internal/aghos/permission_windows_internal_test.go +++ b/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, }} diff --git a/internal/home/home.go b/internal/home/home.go index f79832e5..ed96dee1 100644 --- a/internal/home/home.go +++ b/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) diff --git a/internal/permcheck/migrate.go b/internal/permcheck/migrate.go index ce032f75..65d704c4 100644 --- a/internal/permcheck/migrate.go +++ b/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"))