Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] It's not possible to set UID via file.check_perms when a user can be mapped to two UIDs #62818

Closed
1 task done
rhodesn opened this issue Oct 5, 2022 · 1 comment · Fixed by #62792
Closed
1 task done
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@rhodesn
Copy link
Contributor

rhodesn commented Oct 5, 2022

Description
It's possible to configure a Linux system to pull user information from more than one location. We recently hit an issue where we were pulling the same username from two databases, but will different UIDs; an "old" ID and a "new" ID. Under these circumstances changing file permissions for files owned by that user becomes troublesome, as technically the files are owned by "username", but under the old UID. Salt in file.check_perms converts supplied IDs to names via file.uid_to_user, before comparing existing file permissions to what was requested. The issue with this is that it doesn't spot UID/GID differences, only name differences.

This also applies to GID as well.

Setup

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

  • on-prem machine

Steps to Reproduce the behavior
I only have an example state for a Debian box.

% cat srv/salt/common.sls
passwd-cache-reset:
  file.absent:
    - name: /etc/passwd.cache

nsswitch-reset:
  file.line:
    - name: /etc/nsswitch.conf
    - mode: replace
    - content: "passwd:         files"
    - match: "^passwd:"

dir-absent:
  file.absent:
    - name: /test

testing-create:
  user.present:
    - name: testing
    - uid: 2000
    - usergroup: true

testfile:
  file.managed:
    - name: /test/testfile
    - makedirs: true
    - user: testing
    - group: testing
    - replace: false
    - require:
      - file: dir-absent
      - user: testing-create

libnss-cache:
  pkg.installed

nsswitch-update:
  file.line:
    - name: /etc/nsswitch.conf
    - mode: replace
    - content: "passwd:         cache files"
    - match: "^passwd:"

/etc/passwd.cache:
  file.managed:
    - contents: |
        testing:x:8000:2000::/home/testing:/bin/sh

file_stats_pre:
  module.run:
    - file.stats:
      - path: /test/testfile
    - require:
      - file: /etc/passwd.cache

dir-by-user-new-id:
  file.directory:
    - name: /test
    - user: 8000
    - recurse:
      - user
    - require:
      - file: /etc/passwd.cache

user_info_post:
  module.run:
    - user.info:
      - name: testing
    - require:
      - file: /etc/passwd.cache

file_stats_post:
  module.run:
    - file.stats:
      - path: /test/testfile
    - require:
      - file: testfile

The output from the above is:

# salt-call state.apply common;
local:
----------
          ID: passwd-cache-reset
    Function: file.absent
        Name: /etc/passwd.cache
      Result: True
     Comment: Removed file /etc/passwd.cache
     Started: 09:32:01.306346
    Duration: 3.933 ms
     Changes:
              ----------
              removed:
                  /etc/passwd.cache
----------
          ID: nsswitch-reset
    Function: file.line
        Name: /etc/nsswitch.conf
      Result: True
     Comment: Changes were made
     Started: 09:32:01.310382
    Duration: 2.674 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,4 +1,4 @@
                  -passwd:         cache files
                  +passwd:         files
                   group:          files
                   shadow:         files
                   gshadow:        files
----------
          ID: dir-absent
    Function: file.absent
        Name: /test
      Result: True
     Comment: Removed directory /test
     Started: 09:32:01.313207
    Duration: 0.577 ms
     Changes:
              ----------
              removed:
                  /test
----------
          ID: testing-create
    Function: user.present
        Name: testing
      Result: True
     Comment: User testing is present and up to date
     Started: 09:32:01.314274
    Duration: 11.486 ms
     Changes:
----------
          ID: testfile
    Function: file.managed
        Name: /test/testfile
      Result: True
     Comment: Empty file
     Started: 09:32:01.326149
    Duration: 1.756 ms
     Changes:
              ----------
              group:
                  testing
              new:
                  file /test/testfile created
              user:
                  testing
----------
          ID: libnss-cache
    Function: pkg.installed
      Result: True
     Comment: All specified packages are already installed
     Started: 09:32:02.181527
    Duration: 23.372 ms
     Changes:
----------
          ID: nsswitch-update
    Function: file.line
        Name: /etc/nsswitch.conf
      Result: True
     Comment: Changes were made
     Started: 09:32:02.205086
    Duration: 2.051 ms
     Changes:
              ----------
              diff:
                  ---
                  +++
                  @@ -1,4 +1,4 @@
                  -passwd:         files
                  +passwd:         cache files
                   group:          files
                   shadow:         files
                   gshadow:        files
----------
          ID: /etc/passwd.cache
    Function: file.managed
      Result: True
     Comment: File /etc/passwd.cache updated
     Started: 09:32:02.207244
    Duration: 3.282 ms
     Changes:
              ----------
              diff:
                  New file
----------
          ID: file_stats_pre
    Function: module.run
      Result: True
     Comment: file.stats: Success
     Started: 09:32:02.212124
    Duration: 0.823 ms
     Changes:
              ----------
              file.stats:
                  ----------
                  atime:
                      1664616721.326989
                  ctime:
                      1664616721.326989
                  gid:
                      2000
                  group:
                      testing
                  inode:
                      1318636
                  mode:
                      0644
                  mtime:
                      1664616721.326989
                  size:
                      0
                  target:
                      /test/testfile
                  type:
                      file
                  uid:
                      2000
                  user:
                      testing
----------
          ID: dir-by-user-new-id           <---- ownership changes are returned by
    Function: file.directory                     states.file._check_directory (:748) which  
        Name: /test                              doesn't convert UID -> name before comparing
      Result: True                               user != stats.get("user")
     Comment: Directory /test updated
     Started: 09:32:02.213108
    Duration: 1.702 ms
     Changes:
              ----------
              /test:
                  ----------
                  user:
                      8000
              /test/testfile:
                  ----------
                  user:
                      8000
----------
          ID: user_info_post
    Function: module.run
      Result: True
     Comment: user.info: Success
     Started: 09:32:02.214970
    Duration: 0.928 ms
     Changes:
              ----------
              user.info:
                  ----------
                  fullname:
                  gid:
                      2000
                  groups:
                      - testing
                  home:
                      /home/testing
                  homephone:
                  name:
                      testing
                  other:
                  passwd:
                      x
                  roomnumber:
                  shell:
                      /bin/sh
                  uid:
                      8000
                  workphone:
----------
          ID: file_stats_post
    Function: module.run
      Result: True
     Comment: file.stats: Success
     Started: 09:32:02.216141
    Duration: 0.717 ms
     Changes:
              ----------
              file.stats:
                  ----------
                  atime:
                      1664616721.326989
                  ctime:
                      1664616721.326989
                  gid:
                      2000
                  group:
                      testing
                  inode:
                      1318636
                  mode:
                      0644
                  mtime:
                      1664616721.326989
                  size:
                      0
                  target:
                      /test/testfile
                  type:
                      file
                  uid:                      <---- but not actually implemented
                      2000
                  user:
                      testing

Summary for local
-------------
Succeeded: 12 (changed=10)
Failed:     0
-------------
Total states run:     12
Total run time:   53.301 ms

Expected behavior
I would expect that when running state:

test-perms-by-id:
  file.directory:
    - name: /test
    - user: 8000
    - recurse:
      - user

The directory permissions are recursively updated to UID=8000 which is the UID we want to use for the testing user going forward.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Installing master:

root@minion:/salt# salt --versions-report
Salt Version:
          Salt: 3006.0

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.4
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.15.0
        pygit2: Not Installed
        Python: 3.9.2 (default, Feb 28 2021, 17:03:44)
  python-gnupg: Not Installed
        PyYAML: 6.0
         PyZMQ: 24.0.1
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.124-linuxkit
        system: Linux
       version: Debian GNU/Linux 11 bullseye

root@minion:/salt#

Additional context
Add any other context about the problem here.

@rhodesn rhodesn added Bug broken, incorrect, or confusing behavior needs-triage labels Oct 5, 2022
@welcome
Copy link

welcome bot commented Oct 5, 2022

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant