salt icon indicating copy to clipboard operation
salt copied to clipboard

file module assumes it can do operations as salt user (root) then modify perms

Open ryanwalder opened this issue 8 years ago • 17 comments

Description of Issue/Question

file.managed is unable to write to a nfs mount

Setup

Create an nfs share, mount it on the minion.

file-on-nfs:
  file.managed:
    - name: /path/to/nfs/share/file-i-want-to-manage
    - source: salt://blah/files/file-i-want-to-manage.jinja
    - template: jinja
    - user: username
    - group: groupname
    - mode: 644

Steps to Reproduce Issue

  1. Create nfs share on one VM (or just use one you have)
  2. Mount nfs share on a second VM
  3. Check you can write manually with touch /path/to/nfs/share/test
  4. Try and manage file with salt

Errors

[INFO    ] Running state [/path/to/nfs/share/file-i-want-to-manage] at time 15:20:15.286419
[INFO    ] Executing state file.managed for [/path/to/nfs/share/file-i-want-to-manage]
[DEBUG   ] In saltenv 'base', looking at rel_path 'blah/files/file-i-want-to-manage.jinja' to resolve 'salt://blah/files/file-i-want-to-manage.jinja'
[DEBUG   ] In saltenv 'base', ** considering ** path /path/to/nfs/share/file-i-want-to-manage' to resolve 'salt://blah/files/file-i-want-to-manage.jinja'
[DEBUG   ] Fetching file from saltenv 'base', ** attempting ** 'salt://blah/files/file-i-want-to-manage.jinja'
[DEBUG   ] No dest file found
[INFO    ] Fetching file from saltenv 'base', ** done ** 'blah/files/file-i-want-to-manage.jinja'
[DEBUG   ] Jinja search path: ['/var/cache/salt/minion/files/base']
[DEBUG   ] Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/states/file.py", line 2485, in managed
    **kwargs)
  File "/usr/lib/python2.7/site-packages/salt/modules/file.py", line 5051, in manage_file
    __opts__['cachedir'])
  File "/usr/lib/python2.7/site-packages/salt/utils/files.py", line 87, in copyfile
    tgt = mkstemp(prefix=bname, dir=dname)
  File "/usr/lib/python2.7/site-packages/salt/utils/files.py", line 51, in mkstemp
    return salt.utils.mkstemp(*args, **kwargs)
  File "/usr/lib/python2.7/site-packages/salt/utils/__init__.py", line 3491, in mkstemp
    fd_, fpath = tempfile.mkstemp(*args, **kwargs)
  File "/usr/lib64/python2.7/tempfile.py", line 304, in mkstemp
    return _mkstemp_inner(dir, prefix, suffix, flags)
  File "/usr/lib64/python2.7/tempfile.py", line 239, in _mkstemp_inner
    fd = _os.open(file, flags, 0600)
OSError: [Errno 13] Permission denied: '/path/to/nfs/share/file-i-want-to-manageiAn_w1'

[ERROR   ] Unable to manage file: [Errno 13] Permission denied: /path/to/nfs/share/file-i-want-to-manageAn_w1'
[INFO    ] Completed state [/path/to/nfs/share/file-i-want-to-manage] at time 15:20:15.327657 duration_in_ms=41.237

nfs mount details: 192.168.10.13:/nfs/blah on /path/to/nfs/share type nfs4 (rw,noatime,vers=4.1,rsize=32768,wsize=32768,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=192.168.10.14,lookupcache=pos,local_lock=none,addr=192.168.10.13)

To confirm, I am able to read/write/delete files on the nfs share with the user specified manually, it is only Salt unable to write.

Versions Report

Salt Version:
           Salt: 2017.7.0-n/a-5d719a2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  4 2017, 00:39:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4
 
System Versions:
           dist: centos 7.4.1708 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-693.5.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.4.1708 Core

ryanwalder avatar Dec 18 '17 15:12 ryanwalder

As the error is thrown by OS, a longshot would be SELinux. Is it enabled? Are there any notes about that access in /var/log/secure?

lukasraska avatar Dec 18 '17 16:12 lukasraska

I've disabled SElinux and the issue persists.

No messages in /var/log/messages or /var/log/audit/audit.log.

ryanwalder avatar Dec 18 '17 16:12 ryanwalder

I tried writing the file locally then using file.copy but that also fails, this time with zero error messages in the debug output.

For now I'm writing locally and using a cmd.wait to just cp the file across (as the same user) which works as expected.

ryanwalder avatar Dec 18 '17 16:12 ryanwalder

Was this working previously?

Glancing at the code it looks like it might be trying to create a temp directory on that nfs mount without using the auth of the user.

Ch3LL avatar Dec 18 '17 19:12 Ch3LL

I've not tried using it before on an NFS mount so can't confirm if it worked previously.

From the error I would agree it's during the temp file creation.

ryanwalder avatar Dec 19 '17 10:12 ryanwalder

Just run into the same issue again with file.directory trying to create a dir on an NFS mount. I'm guessing the file module is trying to write as root then chown to the specified user rather than actually executing as the user.

ryanwalder avatar Jan 04 '18 15:01 ryanwalder

Actually after looking at the code it is doing exactly that:

def makedirs_perms(name,
                   user=None,
                   group=None,
                   mode='0755'):
    path = os.path
    head, tail = path.split(name)
    if not tail:
        head, tail = path.split(head)
    if head and tail and not path.exists(head):
        try:
            makedirs_perms(head, user, group, mode)
        except OSError as exc:
            # be happy if someone already created the path
            if exc.errno != errno.EEXIST:
                raise
        if tail == os.curdir:  # xxx/newdir/. exists if xxx/newdir exists
            return
    os.mkdir(name)
    check_perms(name,
                None,
                user,
                group,
int('{0}'.format(mode)) if mode else None)

So the user salt is running as (root in my case) is creating the dir then using check_perms to then set the perms to the specified user rather than creating directly as the user. Which doesn't work if the user running salt can't write to the directory.

ryanwalder avatar Jan 04 '18 15:01 ryanwalder

ahhh that makes sense nice find. Looks like we will need to create the dir as the user specified here. Please feel free to give a go at a PR for this. Thanks for the extra investigation here :)

Ch3LL avatar Jan 04 '18 18:01 Ch3LL

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] avatar Apr 29 '19 19:04 stale[bot]

This is still an issue.

ryanwalder avatar Apr 29 '19 19:04 ryanwalder

Thank you for updating this issue. It is no longer marked as stale.

stale[bot] avatar Apr 29 '19 19:04 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

stale[bot] avatar Jan 08 '20 18:01 stale[bot]

This is still an issue

ryanwalder avatar Jan 08 '20 19:01 ryanwalder

Thank you for updating this issue. It is no longer marked as stale.

stale[bot] avatar Jan 08 '20 19:01 stale[bot]

I think this is an issue with mkstemp.

This also fails with on MacOS read only file system. Minimal code extracted from Salt:

>>> import salt.utils.files
>>> import os
>>> os.getcwd()
'/'
>>> z = salt.utils.files.mkstemp(suffix="", dir=None)
# Salt by default passes in a dir="" which uses the CWD. Use of None lets Python choose.
>>> z = salt.utils.files.mkstemp(suffix="", dir="")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/salt/lib/python3.7/site-packages/salt-3002.5-py3.7.egg/salt/utils/files.py", line 106, in mkstemp
    fd_, f_path = tempfile.mkstemp(*args, **kwargs)
  File "/opt/salt/lib/python3.7/tempfile.py", line 340, in mkstemp
    return _mkstemp_inner(dir, prefix, suffix, flags, output_type)
  File "/opt/salt/lib/python3.7/tempfile.py", line 258, in _mkstemp_inner
    fd = _os.open(file, flags, 0o600)
OSError: [Errno 30] Read-only file system: '__salt.tmp.x7rmqfjp'

Sxderp avatar Aug 10 '21 16:08 Sxderp

This is still an issue.

b-a-t avatar Oct 03 '24 00:10 b-a-t

This is a long-standing issue that is old enough to go to school already. We kind of gave up on it being fixed and learned not to run Salt with CWD on an NFS share and not to manipulate such shares through Salt.

Not sure what was the reason for the failure for the issue originator, but for us it's the NFS option:

 root_squash
              Map  requests  from uid/gid 0 to the anonymous uid/gid. Note that this does not apply to any other uids or gids that might be equally sensitive, such as user bin or group
              staff.

Which we use quite extensively on our NFS shares. So, whenever Salt, which is run as root by default, is trying to create a file or a directory on such a share, it is actually making it as user nobody, who rarely has privileges to do so and can't enforce permissions as root can do on local FSes(if no SELinux or other MAC mechanisms are enabled).

Fixing this issue may require fundamental changes in the file module, as it seems to be a general assumption across the code that root can write whenever (s)he wants.

There is another related problem with this assumption - most of the temp files are created in the CWD without respect to the $TMPDIR, $TEMPDIR or /tmp and /var/tmp. Again, in the majority of the cases, Salt gets away with that inaccuracy, as root can create temporary files almost anywhere on the local FS. But in our case, /home is NFS mounted with root_sqash enabled, so if the user runs sudo salt-call on the system from his home directory - Salt mysteriously fails with Permission denied.

b-a-t avatar Oct 03 '24 00:10 b-a-t