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

salt-ssh temporary files - insecure permissions #40075

Closed
afletch opened this issue Mar 16, 2017 · 8 comments
Closed

salt-ssh temporary files - insecure permissions #40075

afletch opened this issue Mar 16, 2017 · 8 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior fixed-pls-verify fix is linked, bug author to confirm fix P1 Priority 1 Salt-SSH severity-critical top severity, seen by most users, serious issues severity-high 2nd top severity, seen by most users, causes major problems ZRELEASED - 2016.11.4
Milestone

Comments

@afletch
Copy link
Contributor

afletch commented Mar 16, 2017

When salt-ssh sets up it's temporary location (eg. /var/tmp/.root_xxxx_salt), the files contained (eg. /var/tmp/.root_xxxx_salt/running_data/var/cache/salt/minion/files) are 0644. Some of these files may well contain sensitive data such as private keys (which when installed will be set to 0600 by the state).

The permissions may be inherited from the salt-master, but if these files come from a backend such as gitfs, they seem to have 0644 in the master gitfs cache (which in itself is a problem!)

Run the following state with salt-ssh and check the files located in the temporary directory on the remote host. The mykey.key file will be

/etc/pki/tls/private/mykey.key:
  file.managed:
    - source: salt://mystate/files/ssl/mykey.key
    - mode: 0600
    - user: root
    - group: root 
Salt Version:
           Salt: 2016.11.1

Dependency Versions:
           cffi: 1.6.0
       cherrypy: 3.2.2
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: 0.25.1
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: 0.25.0
         Python: 2.7.5 (default, Nov  6 2016, 00:28:07)
   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.3.1611 Core
        machine: x86_64
        release: 3.10.0-229.4.2.el7.x86_64
         system: Linux
        version: CentOS Linux 7.3.1611 Core
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 16, 2017

@afletch looks like i'm able to replicate this behavior on 2016.11.3 and the head of 2016.11. I was not able to replicate this on the head of 2016.3 branch and v2016.3.5 so its good there. We will need to get this fixed thanks.

@Ch3LL Ch3LL added ZRELEASED - 2016.11.4 severity-critical top severity, seen by most users, serious issues Bug broken, incorrect, or confusing behavior severity-high 2nd top severity, seen by most users, causes major problems P1 Priority 1 Salt-SSH labels Mar 16, 2017
@Ch3LL Ch3LL added this to the Approved milestone Mar 16, 2017
@cachedout
Copy link
Contributor

@gtmanfred I have assigned this over to you. Please also ensure that a test is written for this so that it does not regress prior to closing this.

@gtmanfred
Copy link
Contributor

This is going to require some reworking of how the fileclient works.

Because of the change made here, 73a156d

We mirror the file permissions down to the minion so that we can keep the same permissions as the fileserver if the mode and owner/group are not set.

@terminalmage
Copy link
Contributor

It's not that we mirror the mode when not set, it's when explicitly configured to do so using keep as the mode.

@terminalmage
Copy link
Contributor

When no mode is set, we let the system umask decide.

@gtmanfred
Copy link
Contributor

This should be fixed by PR #40609

It does mean that we cannot use the mode: keep when using salt-ssh.

Thanks,
Daniel

@Ch3LL
Copy link
Contributor

Ch3LL commented Apr 11, 2017

Closing since this has been resolved.

@Ch3LL Ch3LL closed this as completed Apr 11, 2017
@msmeissn
Copy link

assigned CVE-2017-8109.

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 fixed-pls-verify fix is linked, bug author to confirm fix P1 Priority 1 Salt-SSH severity-critical top severity, seen by most users, serious issues severity-high 2nd top severity, seen by most users, causes major problems ZRELEASED - 2016.11.4
Projects
None yet
Development

No branches or pull requests

6 participants