Issue #8770 has been updated by Nick Lewis. Status changed from Merged - Pending Release to In Topic Branch Pending Merge
Fixed an issue with this change in https://github.com/puppetlabs/puppet/pull/37. Puppet::Util::SUIDManager.change_user was always trying to set supplementary groups (Process.initgroups) before changing its EUID. Process.initgroups requires the calling process to have EUID 0 in order to succeed. This worked fine in the case where the process was changing from root to a normal user, as it would set groups as root and then change EUID to 0. However, in the case where the process was changing back to root from a normal user, it would attempt to set groups as the normal user, and fail. Now, we check Process.euid before changing, and will set groups first if root, and will set euid first if not root. This ensures we can freely switch back and forth between root. This behavior is maintained inside of the change_user, rather than being broken into eg. raise_privilege and lower_privilege, because it is a relatively minor behavior difference, and the helper methods on their own would not have been generically useful. ---------------------------------------- Bug #8770: Puppetmaster does not drop privileges on Mac OSX https://projects.puppetlabs.com/issues/8770 Author: Josh Cooper Status: In Topic Branch Pending Merge Priority: High Assignee: Nick Lewis Category: security Target version: 2.6.x Affected Puppet version: Keywords: osx Branch: On Mac OSX, the ruby methods Process.uid= and Process.gid= do not actually change the real UID/GID. In other words, they change the uid/gid values that ruby stores internally, but the calls never result in the corresponding system calls (setuid/setgid). To work around this problem, Puppet::Util::SUIDManager circa commit:6d8068eddd0d29ec53f62557eb53f6ebb8e40591 defined uid/gid setter methods that forward to the effective uid/gid setter methods in the Process module when running on Mac OSX. <pre> if platform == "Darwin" and (method == :uid= or method == :gid=) Puppet::Util::Warnings.warnonce "Cannot change real UID on Darwin" newmethod = ("e" + method.to_s).intern end </pre> However, if a process running as root calls SUIDManager.uid=X to drop privileges, which puppetmaster does via Puppet::Util.chuser, the process can trivially regain root privileges, since its real uid was never changed. Nick has example code that does this. In addition, this commit:8f059510ae0c2b690b3b800778ca765860645357 commit refactored the code that handled the uid/gid setter methods, but in doing so, it aliased the uid/gid getter methods: <pre> if Facter['kernel'].value == 'Darwin' # Cannot change real UID on Darwin so we set euid alias :uid :euid alias :gid :egid end </pre> To fix this on Mac OSX we should be using Process::UID.change_privilege(new_uid) and Process::GID.change_privilege(new_gid). For example, the following code correctly sets the real UID to a non-root user, resulting in a permission error when attempting to switch back to root: <pre> $ sudo ruby -e "Process::UID.change_privilege(502); Process.uid=0; puts \"uid=#{Process.uid} euid=#{Process.euid}\"" -e:1:in `uid=': Operation not permitted (Errno::EPERM) from -e:1 </pre> This needs to be fixed in the next 2.6.x release and later. Note it is not an issue for Mac OSX agents, because they run as root and do not drop privileges. -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
