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

highstate.cache is world readable, and contains secrets #28455

Closed
zmalone opened this issue Oct 30, 2015 · 8 comments
Closed

highstate.cache is world readable, and contains secrets #28455

zmalone opened this issue Oct 30, 2015 · 8 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix P1 Priority 1 severity-critical top severity, seen by most users, serious issues severity-high 2nd top severity, seen by most users, causes major problems
Milestone

Comments

@zmalone
Copy link
Contributor

zmalone commented Oct 30, 2015

$ salt-minion --version
salt-minion 2015.5.3 (Lithium)
$ ls -l /var/cache/salt/minion/highstate.cache.p
-rw-r--r-- 1 root root 5215 Oct 28 19:53 /var/cache/salt/minion/highstate.cache.p
$ strings /var/cache/salt/minion/highstate.cache.p
user
<username>
password
<password hash>

Secrets get dumped in highstate.cache.p, which is world readable. It looks like this has been fixed before, so this is probably a regression. See #6764 for a past instance of this.

This also appears in 2015.8.1:

vagrant@vm:~$ salt-minion --version
salt-minion 2015.8.1 (Beryllium)
vagrant@vm:~$ ls -l /var/cache/salt/minion/highstate.cache.p
-rw-r--r-- 1 root root 2539 Oct 29 21:32 /var/cache/salt/minion/highstate.cache.p
@pcn
Copy link
Contributor

pcn commented Oct 30, 2015

I'm not sure what's different, but I checked this out because I was concerned about this, and the cache on all of the minions I have (2015.8.1 and some older 2015.5.3 minions) are 0600.

Also, is the password hash exactly a secret on the node that it's being installed on?

@cachedout
Copy link
Contributor

Hi @zmalone.

I can only reproduce this (thus far) with state.sls. It appears as though state.highstate is not affected by this problem. Can you confirm?

@zmalone
Copy link
Contributor Author

zmalone commented Oct 30, 2015

I can confirm that I'm using state.sls. I'm attempting to test with highstates now, but it sounds as though that code path does not have this issue. I've worked around it for now with:

file.managed:
    - name: /var/cache/salt/minion/highstate.cache.p
    - mode: 0600

but it will probably effect a lot of people who run state.sls.

@cachedout
Copy link
Contributor

I already have a fix in for state.sls over in #28461. I'll review the rest of state.py right now to ensure there aren't any other mistakes of this nature. Thanks for bringing this one to our attention. Much appreciated.

@zmalone
Copy link
Contributor Author

zmalone commented Oct 30, 2015

I've confirmed that I do not see this with state.highstate.

@cachedout
Copy link
Contributor

@zmalone I can also confirm that. I believe this problem to be isolated to state.sls and fixed by the PR linked above.

@basepi basepi added Bug broken, incorrect, or confusing behavior fixed-pls-verify fix is linked, bug author to confirm fix Core relates to code central or existential to Salt severity-high 2nd top severity, seen by most users, causes major problems severity-critical top severity, seen by most users, serious issues P2 Priority 2 P1 Priority 1 and removed P2 Priority 2 labels Oct 30, 2015
@basepi basepi added this to the B 9 milestone Oct 30, 2015
@cachedout
Copy link
Contributor

This fix has been merged. I'm going to go ahead close this unless there is additional work to be done here.

@basepi
Copy link
Contributor

basepi commented Dec 3, 2015

@zmalone The CVE for this fix is being announced today. In the future, please send security concerns to security@saltstack.com as instructed in our security disclosure documentation: https://docs.saltstack.com/en/latest/security/index.html

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

No branches or pull requests

4 participants