(#8770) Don't fail to set supplementary groups when changing user to root

Previously, Puppet::Util::SUIDManager.change_user would always try 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.

1 files changed, 10 insertions, 2 deletions

diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index 2e12b220f..697bce111 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -82,13 +82,21 @@ module Puppet::Util::SUIDManager
       begin
         Process::UID.change_privilege(uid)
       rescue NotImplementedError
+        # If changing uid, we must be root. So initgroups first here.
         initgroups(uid)
         Process.euid = uid
         Process.uid  = uid
       end
     else
-      initgroups(uid)
-      Process.euid = uid
+      # If we're already root, initgroups before changing euid. If we're not,
+      # change euid (to root) first.
+      if Process.euid == 0
+        initgroups(uid)
+        Process.euid = uid
+      else
+        Process.euid = uid
+        initgroups(uid)
+      end
     end
   end
   module_function :change_user