saltstack / saltstack/salt

[BUG] file.directory.win_perms doesn't correct permissions in case of deviating permission even with win_perms_reset

Open
#64,006 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
the file.directory.win_perms and win_perms_reset don't set the correct permissions in case of deviating permissions.

Explanation:
I have a folder called testdir in C:\Admin, so C:\Admin\testdir
On this System I have the following (wrong) permissions set (see Screenshot "Wrong Permission" for visual)

allow, SYSTEM, Full Control, NONE inheritance, this folder subfolders and files
allow, Administrators, Full Control, NONE inheritance, this folder subfolders and files
allow, testuser, Full Control, NONE inheritance, this folder subfolders and files

Testuser, however isn't allowed to have "this folder, subfolders and files". I want to correct deviating permissions with a state.

The correct permission would be like this (see Screenshot "Correct and expected permissions" for visual)
allow, SYSTEM, Full Control, NONE inheritance, this folder subfolders and files
allow, Administrators, Full Control, NONE inheritance, this folder subfolders and files
allow, testuser, Full Control, NONE inheritance, subfolders and files

Setup
States used to reproduce this

# Make sure C:\Admin exists
{{ sls }}_adminFolderExists: 
  file.directory: 
    - name: 'C:\Admin' 
    - win_perms_reset: true 
    - win_owner: Administrators 
    - win_perms: 
        Administrators: 
            perms: full_control 
            applies_to: this_folder_subfolder_files
        SYSTEM: 
            perms: full_control 
            applies_to: this_folder_subfolder_files 
    - win_inheritance: false

# Make sure C:\Admin\testdir exists
{{ sls }}_testdirFolderExists: 
  file.directory: 
    - name: 'C:\Admin\testdir\' 
    - win_perms_reset: true 
    - win_owner: Administrators 
    - win_perms: 
        Administrators: 
            perms: full_control  
        SYSTEM: 
            perms: full_control
        testuser: 
            perms: full_control 
            applies_to: subfolders_files
    - win_inheritance: false

debug log:

[PROFILE ] Time (in seconds) to render 'C:\ProgramData\Salt 
Project\Salt\var\cache\salt\minion\files\base\windows_default\states\files_and_directories\init.sls' using 'yaml' renderer: 0.006020784378051758
[DEBUG   ] The functions from module 'config' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded config.option
[DEBUG   ] The functions from module 'file' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded file.directory
[INFO    ] Running state [C:\Admin] at time 11:20:41.541873
[INFO    ] Executing state file.directory for [C:\Admin]
[DEBUG   ] Resetting permissions for C:\Admin
[DEBUG   ] Getting current permissions for C:\Admin
[INFO    ] The directory C:\Admin is in the correct state
[INFO    ] Completed state [C:\Admin] at time 11:20:41.564052 (duration_in_ms=22.179)
[INFO    ] Running state [C:\Admin\testdir\] at time 11:20:41.659060
[INFO    ] Executing state file.directory for [C:\Admin\testdir\]
[DEBUG   ] Resetting permissions for C:\Admin\testdir\
[DEBUG   ] Getting current permissions for C:\Admin\testdir\
[INFO    ] The directory C:\Admin\testdir\ is in the correct state
[INFO    ] Completed state [C:\Admin\testdir\] at time 11:20:41.690758 (duration_in_ms=31.698)
[DEBUG   ] File C:\ProgramData\Salt Project\Salt\var\cache\salt\minion\accumulator\1985296711632 does not exist, no need to cleanup
[DEBUG   ] The functions from module 'cmd' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded cmd.run
[INFO    ] Executing command attrib in directory 'C:\Users\Administrator'
[DEBUG   ] output: 
[DEBUG   ] The functions from module 'state' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded state.check_result
[DEBUG   ] The functions from module 'highstate' are being loaded by dir() on the loaded module
[DEBUG   ] LazyLoaded highstate.output
local:

          ID: windows_default.states.files_and_directories_adminFolderExists
    Function: file.directory
        Name: C:\Admin
      Result: True
     Comment: The directory C:\Admin is in the correct state
     Started: 11:20:41.541873
    Duration: 22.179 ms
     Changes:   
----------
          ID: windows_default.states.files_and_directories_testdirFolderExists
    Function: file.directory
        Name: C:\Admin\testdir\
      Result: True
     Comment: The directory C:\Admin\testdir\ is in the correct state
     Started: 11:20:41.659060
    Duration: 31.698 ms
     Changes:   

Please be as specific as possible and give set-up details.

  • [ x] VM (VMWare) - Windows Server 2022

Steps to Reproduce the behavior
There are no errors. It even says the suspected folder is in correct state even though the permissions of testuser are deviating.
Comment: The directory C:\Admin\testdir\ is in the correct state

To reproduce this, a user needs to have full control on a folder with "applies to" this folder, subfolders and files (see the screenshot "Wrong Permissions").
The state needs to look like the given state above.

Expected behavior
The expected behavior is, that the testuser in this case loses it's permission of "full control" applies to "this folder, subfolders and files" and instead is granted a permission of "full control" applies to "subfolders and files"

Screenshots
Wrong Permission:
grafik

Correct and expected permissions:
grafik

Versions Report

salt --versions-report Salt Version:           Salt: 3005.1

Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.7
     gitpython: Not Installed
        Jinja2: 3.1.0
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: 1.1.4
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.10.1
        pygit2: Not Installed
        Python: 3.8.16 (tags/v3.8.16:1e3d2d5, Jan  6 2023, 17:39:57) [MSC v.1929 64 bit (AMD64)]
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 22.0.3
         smmap: 4.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist:   
        locale: cp1252
       machine: AMD64
       release: 2022Server
        system: Windows
       version: 2022Server 10.0.20348 SP0 Multiprocessor Free

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the file.directory entry point and trace handling for win_perms_reset, win_perms, and applies_to on Windows. Reproduce the provided C:\Admin\testdir scenario on Windows Server 2022, then verify that a deviating testuser ACL is detected and corrected to subfolders_files without reporting the directory as already correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
infrastructure, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.