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

publish_session does not update the encryption key #11493

Closed
jcsp opened this issue Mar 25, 2014 · 1 comment
Closed

publish_session does not update the encryption key #11493

jcsp opened this issue Mar 25, 2014 · 1 comment

Comments

@jcsp
Copy link
Contributor

jcsp commented Mar 25, 2014

The following commit broke publish_session:

commit cf7e116796ccaf0aef76b4dad2b6c6908aee6d61
Author: Thomas S Hatch <thatch45@gmail.com>
Date:   Sat Jun 15 11:34:13 2013 -0600

    Set user of the running master

Specifically, in crypt.dropfile:

+    if user:
+        try:
+            import pwd
+            uid = pwd.getpwnam(user).pw_uid
+            os.chown(dfnt, uid, -1)
+            shutil.move(dfnt, dfn)
+        except (KeyError, ImportError, OSError, IOError):
+            pass

The shutil.move is inside the "if user" conditional. User is None when dropfile is used the call from master.py, so the .dfnt file is written but never renamed to .dfn, so the master never sees it and the key never gets updated.

This is an indirect security bug, if a deployment is relying on publish_session to increase the difficulty of attacks.

jcsp pushed a commit to jcsp/salt that referenced this issue Mar 25, 2014
crypt.dropfile was failing to emplace the .dfn file
when user was None, as is the case in the periodic
call based on the publish_session config setting.
@jcsp
Copy link
Contributor Author

jcsp commented Mar 25, 2014

Please consider this for backport as it is security related.

thatch45 added a commit that referenced this issue Mar 25, 2014
thatch45 added a commit that referenced this issue Mar 25, 2014
@basepi, please cherrypick
basepi pushed a commit that referenced this issue Apr 4, 2014
crypt.dropfile was failing to emplace the .dfn file
when user was None, as is the case in the periodic
call based on the publish_session config setting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant