(#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>

2 files changed, 56 insertions, 54 deletions

diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 34c6ec1ed..72c1f5f91 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -46,35 +46,24 @@ module Util
   # Change the process to a different user
   def self.chuser
     if group = Puppet[:group]
-      group = self.gid(group)
-      raise Puppet::Error, "No such group #{Puppet[:group]}" unless group
-      unless Puppet::Util::SUIDManager.gid == group
-        begin
-          Puppet::Util::SUIDManager.egid = group
-          Puppet::Util::SUIDManager.gid = group
-        rescue => detail
-          Puppet.warning "could not change to group #{group.inspect}: #{detail}"
-          $stderr.puts "could not change to group #{group.inspect}"
-
-          # Don't exit on failed group changes, since it's
-          # not fatal
-          #exit(74)
-        end
+      begin
+        Puppet::Util::SUIDManager.change_group(group, true)
+      rescue => detail
+        Puppet.warning "could not change to group #{group.inspect}: #{detail}"
+        $stderr.puts "could not change to group #{group.inspect}"
+
+        # Don't exit on failed group changes, since it's
+        # not fatal
+        #exit(74)
       end
     end
 
     if user = Puppet[:user]
-      user = self.uid(user)
-      raise Puppet::Error, "No such user #{Puppet[:user]}" unless user
-      unless Puppet::Util::SUIDManager.uid == user
-        begin
-          Puppet::Util::SUIDManager.initgroups(user)
-          Puppet::Util::SUIDManager.uid = user
-          Puppet::Util::SUIDManager.euid = user
-        rescue => detail
-          $stderr.puts "Could not change to user #{user}: #{detail}"
-          exit(74)
-        end
+      begin
+        Puppet::Util::SUIDManager.change_user(user, true)
+      rescue => detail
+        $stderr.puts "Could not change to user #{user}: #{detail}"
+        exit(74)
       end
     end
   end
@@ -89,18 +78,14 @@ module Util
         if useself
 
           Puppet::Util::Log.create(
-
             :level => level,
             :source => self,
-
             :message => args
           )
         else
 
           Puppet::Util::Log.create(
-
             :level => level,
-
             :message => args
           )
         end
@@ -261,9 +246,6 @@ module Util
       Puppet.debug "Executing '#{str}'"
     end
 
-    arguments[:uid] = Puppet::Util::SUIDManager.convert_xid(:uid, arguments[:uid]) if arguments[:uid]
-    arguments[:gid] = Puppet::Util::SUIDManager.convert_xid(:gid, arguments[:gid]) if arguments[:gid]
-
     if execution_stub = Puppet::Util::ExecutionStub.current_value
       return execution_stub.call(command, arguments)
     end
@@ -305,14 +287,8 @@ module Util
           $stderr.reopen(error_file)
 
           3.upto(256){|fd| IO::new(fd).close rescue nil}
-          if arguments[:gid]
-            Process.egid = arguments[:gid]
-            Process.gid = arguments[:gid] unless @@os == "Darwin"
-          end
-          if arguments[:uid]
-            Process.euid = arguments[:uid]
-            Process.uid = arguments[:uid] unless @@os == "Darwin"
-          end
+          Puppet::Util::SUIDManager.change_group(arguments[:gid], true) if arguments[:gid]
+          Puppet::Util::SUIDManager.change_user(arguments[:uid], true) if arguments[:uid]
           ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C'
           if command.is_a?(Array)
             Kernel.exec(*command)
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}