(#8770) Always fully drop privileges when changing user

On Mac OS X, it is only possible to directly change the euid of a process, and
not the uid. Thus, when a puppet master started as root on OS X would change to
the service user (puppet), it would leave the uid of its process set to 0.
This allowed any type of Ruby plugin executed on the master (a type, provider,
function, etc.) to trivially regain root privileges (by setting the euid of
its process back to 0) and potentially compromise the master.

Now, when permanently changing user, we will first try
Process::UID.change_privilege, before falling back to setting the euid/uid
ourselves. change_privilege correctly sets the uid of the process to the
desired new uid, preventing the process from later escalating itself back to
root. Similar behavior is also used when changing group. This has no effect on
the behavior when temporarily changing user/group (for instance, to execute a
single command or create a file as a particular user).

Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>

1 files changed, 40 insertions, 14 deletions

diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index 6633de002..2e12b220f 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -36,12 +36,6 @@ module Puppet::Util::SUIDManager
   end
   module_function :groups=
 
-  if Facter['kernel'].value == 'Darwin'
-    # Cannot change real UID on Darwin so we set euid
-    alias :uid :euid
-    alias :gid :egid
-  end
-
   def self.root?
     Process.uid == 0
   end
@@ -50,23 +44,55 @@ module Puppet::Util::SUIDManager
   def asuser(new_uid=nil, new_gid=nil)
     return yield if Puppet.features.microsoft_windows? or !root?
 
-    # We set both because some programs like to drop privs, i.e. bash.
-    old_uid, old_gid = self.uid, self.gid
     old_euid, old_egid = self.euid, self.egid
-    old_groups = self.groups
     begin
-      self.egid = convert_xid :gid, new_gid if new_gid
-      self.initgroups(convert_xid(:uid, new_uid)) if new_uid
-      self.euid = convert_xid :uid, new_uid if new_uid
+      change_group(new_gid) if new_gid
+      change_user(new_uid) if new_uid
 
       yield
     ensure
-      self.euid, self.egid = old_euid, old_egid
-      self.groups = old_groups
+      change_group(old_egid)
+      change_user(old_euid)
     end
   end
   module_function :asuser
 
+  def change_group(group, permanently=false)
+    gid = convert_xid(:gid, group)
+    raise Puppet::Error, "No such group #{group}" unless gid
+
+    if permanently
+      begin
+        Process::GID.change_privilege(gid)
+      rescue NotImplementedError
+        Process.egid = gid
+        Process.gid  = gid
+      end
+    else
+      Process.egid = gid
+    end
+  end
+  module_function :change_group
+
+  def change_user(user, permanently=false)
+    uid = convert_xid(:uid, user)
+    raise Puppet::Error, "No such user #{user}" unless uid
+
+    if permanently
+      begin
+        Process::UID.change_privilege(uid)
+      rescue NotImplementedError
+        initgroups(uid)
+        Process.euid = uid
+        Process.uid  = uid
+      end
+    else
+      initgroups(uid)
+      Process.euid = uid
+    end
+  end
+  module_function :change_user
+
   # Make sure the passed argument is a number.
   def convert_xid(type, id)
     map = {:gid => :group, :uid => :user}