+ Puppet::SUIDManager - This replaces all calls to the built-in ruby 'Process' library for uid/gid/euid/egid operations, including (not surprisingly) Puppet::Util#asuser and a method to run commands and capture output. This is due to many inconsistencies (through bugfixes) between ruby versions in the 1.8.x branch. This is included in the core puppet library and can be used by all puppet types and providers.

! Modified Puppet::Util#uid to check (and warn) if passed a nil value.
! Changes to use Puppet::SUIDManager instead of Process and relevant Puppet::Util calls.
! Removed Puppet::Util#asuser.


git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1666 980ebf18-57e1-0310-9a29-db15c13687c0

12 files changed, 109 insertions, 121 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index f6debc0b1..e0d8a8e6c 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -5,6 +5,7 @@ require 'puppet/event-loop'
 require 'puppet/util'
 require 'puppet/log'
 require 'puppet/config'
+require 'puppet/suidmanager'
 
 #------------------------------------------------------------
 # the top-level module
@@ -74,7 +75,7 @@ module Puppet
     # use basedirs that are in the user's home directory.
     conf = nil
     var = nil
-    if self.name == "puppet" and Process.uid != 0
+    if self.name == "puppet" and Puppet::SUIDManager.uid != 0
         conf = File.expand_path("~/.puppet")
         var = File.expand_path("~/.puppet/var")
     else
diff --git a/lib/puppet/config.rb b/lib/puppet/config.rb
index 81ec4fe85..337a50bbc 100644
--- a/lib/puppet/config.rb
+++ b/lib/puppet/config.rb
@@ -203,7 +203,7 @@ class Config
             raise ArgumentError, "Default %s is not a file" % default
         end
 
-        Puppet::Util.asuser(obj.owner, obj.group) do
+        Puppet::SUIDManager.asuser(obj.owner, obj.group) do
             mode = obj.mode || 0750
             Dir.mkdir(obj.value, mode)
         end
@@ -629,12 +629,12 @@ Generated on #{Time.now}.
         end
 
         chown = nil
-        if Process.uid == 0
+        if Puppet::SUIDManager.uid == 0
             chown = [obj.owner, obj.group]
         else
             chown = [nil, nil]
         end
-        Puppet::Util.asuser(*chown) do
+        Puppet::SUIDManager.asuser(*chown) do
             mode = obj.mode || 0640
 
             if args.empty?
@@ -662,13 +662,13 @@ Generated on #{Time.now}.
         end
 
         chown = nil
-        if Process.uid == 0
+        if Puppet::SUIDManager.uid == 0
             chown = [obj.owner, obj.group]
         else
             chown = [nil, nil]
         end
 
-        Puppet::Util.asuser(*chown) do
+        Puppet::SUIDManager.asuser(*chown) do
             mode = obj.mode || 0640
             if args.empty?
                 args << "w"
@@ -878,7 +878,7 @@ Generated on #{Time.now}.
             }
 
             # Only chown or chgrp when root
-            if Process.uid == 0
+            if Puppet::SUIDManager.uid == 0
                 [:group, :owner].each { |var|
                     if value = self.send(var)
                         obj[var] = value
diff --git a/lib/puppet/filetype.rb b/lib/puppet/filetype.rb
index 081448f33..d05c1469d 100755
--- a/lib/puppet/filetype.rb
+++ b/lib/puppet/filetype.rb
@@ -174,7 +174,7 @@ module Puppet
             # does not think I should be allowed to set the @path to my own user name
             def cmdbase
                 cmd = nil
-                if @uid == Process.uid
+                if @uid == Puppet::SUIDManager.uid
                     return "crontab"
                 else
                     return "crontab -u #{@path}"
@@ -187,14 +187,14 @@ module Puppet
         newfiletype(:suntab) do
             # Read a specific @path's cron tab.
             def read
-                Puppet::Util.asuser(@path) {
+                Puppet::SUIDManager.asuser(@path) {
                     %x{crontab -l 2>/dev/null}
                 }
             end
 
             # Remove a specific @path's cron tab.
             def remove
-                Puppet::Util.asuser(@path) {
+                Puppet::SUIDManager.asuser(@path) {
                     %x{crontab -r 2>/dev/null}
                 }
             end
@@ -202,7 +202,7 @@ module Puppet
             # Overwrite a specific @path's cron tab; must be passed the @path name
             # and the text with which to create the cron tab.
             def write(text)
-                Puppet::Util.asuser(@path) {
+                Puppet::SUIDManager.asuser(@path) {
                     IO.popen("crontab", "w") { |p|
                         p.print text
                     }
diff --git a/lib/puppet/sslcertificates/ca.rb b/lib/puppet/sslcertificates/ca.rb
index b1c5b34e6..19ea27228 100644
--- a/lib/puppet/sslcertificates/ca.rb
+++ b/lib/puppet/sslcertificates/ca.rb
@@ -265,7 +265,7 @@ class Puppet::SSLCertificates::CA
         )
 
         # This creates the cakey file
-        Puppet::Util.asuser(Puppet[:user], Puppet[:group]) do
+        Puppet::SUIDManager.asuser(Puppet[:user], Puppet[:group]) do
             @cert = cert.mkselfsigned
         end
         Puppet.config.write(:cacert) do |f|
diff --git a/lib/puppet/suidmanager.rb b/lib/puppet/suidmanager.rb
new file mode 100644
index 000000000..2f4d428e3
--- /dev/null
+++ b/lib/puppet/suidmanager.rb
@@ -0,0 +1,74 @@
+require 'facter'
+require 'puppet'
+
+module Puppet
+    module SUIDManager
+        platform = Facter["kernel"].value
+        [:uid=, :uid, :gid=, :gid].each do |method|
+            define_method(method) do |*args|
+                if platform == "Darwin" and (Facter['rubyversion'] <=> "1.8.5") < 0
+                    Puppet.warning "Cannot change real UID on Darwin on Ruby versions earlier than 1.8.5"
+                    method = ("e" + method.to_s).intern unless method.to_s[0] == 'e'
+                end
+
+                return Process.send(method, *args)
+            end
+            module_function method
+        end
+
+        [:euid=, :euid, :egid=, :egid].each do |method|
+            define_method(method) do |*args|
+                Process.send(method, *args)
+            end
+            module_function method
+        end
+
+        def run_and_capture(command, new_uid=self.euid, new_gid=self.egid)
+            output = nil
+
+            asuser(new_uid, new_gid) do
+                # capture both stdout and stderr unless we are on ruby < 1.8.4
+                # NOTE: this would be much better facilitated with a specialized popen()
+                #       (see the test suite for more details.)
+                if (Facter['rubyversion'].value <=> "1.8.4") < 0
+                   unless @@alreadywarned
+                        Puppet.warning "Cannot capture STDERR when running as another user on Ruby < 1.8.4"
+                        @@alreadywarned = true
+                    end
+                    output = %x{#{command}}
+                else
+                    output = %x{#{command} 2>&1}
+                end
+            end
+
+            [output, $?.dup]
+        end
+
+        module_function :run_and_capture
+
+        def system(command, new_uid=self.euid, new_gid=self.egid)
+            asuser(new_uid, new_gid) do
+                Kernel.system(command)
+            end
+        end
+        
+        module_function :system
+
+        def asuser(new_euid, new_egid)
+            new_euid = Puppet::Util.uid(new_euid)
+            new_egid = Puppet::Util.uid(new_egid)
+
+            old_euid, old_egid = [ self.euid, self.egid ]
+            self.egid = new_egid ? new_egid : old_egid
+            self.euid = new_euid ? new_euid : old_euid
+            output = yield
+            self.egid = old_egid
+            self.euid = old_euid
+
+            output
+        end
+
+        module_function :asuser
+    end
+end
+
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 442eb311f..8b964cbb3 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -186,7 +186,7 @@ module Puppet
                 is because of a bug within Ruby."
 
             munge do |user|
-                unless Process.uid == 0
+                unless Puppet::SUIDManager.uid == 0
                     self.fail "Only root can execute commands as other users"
                 end
                 require 'etc'
@@ -537,26 +537,9 @@ module Puppet
                     end
 
                     withenv env do
-                        # The user and group default to nil, which 'asuser'
-                        # handlers correctly
-                        Puppet::Util.asuser(self[:user], self[:group]) {
-                            # capture both stdout and stderr
-                            if self[:user]
-                                unless defined? @@alreadywarned
-                                    Puppet.warning(
-                            "Cannot capture STDERR when running as another user"
-                                    )
-                                    @@alreadywarned = true
-                                end
-                                output = %x{#{command}}
-                            else
-                                output = %x{#{command} 2>&1}
-                            end
-                        }
-                        status = $?.dup
-
+                        output, status = Puppet::SUIDManager.run_and_capture(command, self[:user], self[:group])
                         # The shell returns 127 if the command is missing.
-                        if $?.exitstatus == 127
+                        if status.exitstatus == 127
                             raise ArgumentError, output
                         end
                     end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 7ce384077..5d7a3e881 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -226,7 +226,7 @@ module Puppet
         # Determine the user to write files as.
         def asuser
             if self.should(:owner) and ! self.should(:owner).is_a?(Symbol)
-                writeable = Puppet::Util.asuser(self.should(:owner)) {
+                writeable = Puppet::SUIDManager.asuser(self.should(:owner)) {
                     FileTest.writable?(File.dirname(self[:path]))
                 }
 
@@ -930,7 +930,7 @@ module Puppet
             end
 
             # As the correct user and group
-            Puppet::Util.asuser(asuser(), self.should(:group)) do
+            Puppet::SUIDManager.asuser(asuser(), self.should(:group)) do
                 f = nil
                 # Open our file with the correct modes
                 if mode
diff --git a/lib/puppet/type/pfile/ensure.rb b/lib/puppet/type/pfile/ensure.rb
index ac045dfd6..2e48e0165 100755
--- a/lib/puppet/type/pfile/ensure.rb
+++ b/lib/puppet/type/pfile/ensure.rb
@@ -67,7 +67,7 @@ module Puppet
                     "Cannot create %s; parent directory %s does not exist" %
                         [@parent[:path], parent]
             end
-            Puppet::Util.asuser(@parent.asuser()) {
+            Puppet::SUIDManager.asuser(@parent.asuser()) {
                 if mode
                     Puppet::Util.withumask(000) do
                         Dir.mkdir(@parent[:path],mode)
diff --git a/lib/puppet/type/pfile/source.rb b/lib/puppet/type/pfile/source.rb
index 65aec1dfd..9ee236850 100755
--- a/lib/puppet/type/pfile/source.rb
+++ b/lib/puppet/type/pfile/source.rb
@@ -71,7 +71,7 @@ module Puppet
             }
 
             # we can't manage ownership as root, so don't even try
-            unless Process.uid == 0
+            unless Puppet::SUIDManager.uid == 0
                 args.delete(:owner)
             end
 
diff --git a/lib/puppet/type/pfile/target.rb b/lib/puppet/type/pfile/target.rb
index 23fb30390..a2d174c2e 100644
--- a/lib/puppet/type/pfile/target.rb
+++ b/lib/puppet/type/pfile/target.rb
@@ -45,7 +45,7 @@ module Puppet
                 end
             end
             Dir.chdir(File.dirname(@parent[:path])) do
-                Puppet::Util.asuser(@parent.asuser()) do
+                Puppet::SUIDManager.asuser(@parent.asuser()) do
                     mode = @parent.should(:mode)
                     if mode
                         Puppet::Util.withumask(000) do
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index 166adac32..72d2a7e03 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -117,7 +117,7 @@ module Puppet
         end
 
         def sync
-            unless Process.uid == 0
+            unless Puppet::SUIDManager.uid == 0
                 unless defined? @@notifieduid
                     self.notice "Cannot manage ownership unless running as root"
                     #@parent.delete(self.name)
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index c6ad30e3b..049d66b49 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -19,83 +19,6 @@ module Util
         return @@syncresources[resource]
     end
 
-    # Execute a block as a given user or group
-    def self.asuser(user = nil, group = nil)
-        require 'etc'
-
-        uid = nil
-        gid = nil
-        olduid = nil
-        oldgid = nil
-
-        # If they're running as a normal user, then just execute as that same
-        # user.
-        unless Process.uid == 0
-            retval = yield
-            return retval
-        end
-
-        begin
-            # the groupid, if we got passed a group
-            # The gid has to be changed first, because, well, otherwise we won't
-            # be able to
-            if group
-                if group.is_a? Integer
-                    gid = group
-                else
-                    gid = self.gid(group)
-                end
-
-                if gid
-                    if Process.gid != gid
-                        oldgid = Process.gid
-                        begin
-                            Process.egid = gid
-                        rescue => detail
-                            raise Puppet::Error, "Could not change GID: %s" % detail
-                        end
-                    end
-                else
-                    Puppet.warning "Could not retrieve GID for %s" % group
-                end
-            end
-
-            if user
-                if user.is_a? Integer
-                    uid = user
-                else
-                    uid = self.uid(user)
-                end
-                uid = self.uid(user)
-
-                if uid
-                    # Now change the uid
-                    if Process.uid != uid
-                        olduid = Process.uid
-                        begin
-                            Process.euid = uid
-                        rescue => detail
-                            raise Puppet::Error, "Could not change UID: %s" % detail
-                        end
-                    end
-                else
-                    Puppet.warning "Could not retrieve UID for %s" % user
-                end
-            end
-            retval = yield
-        ensure
-            if olduid
-                Process.euid = olduid
-            end
-
-            if oldgid
-                Process.egid = oldgid
-            end
-        end
-
-        return retval
-    end
-
     # Change the process to a different user
     def self.chuser
         if Facter["operatingsystem"].value == "Darwin"
@@ -107,10 +30,10 @@ module Util
             unless group
                 raise Puppet::Error, "No such group %s" % Puppet[:group]
             end
-            unless Process.gid == group
+            unless Puppet::SUIDManager.gid == group
                 begin
-                    Process.egid = group 
-                    Process.gid = group 
+                    Puppet::SUIDManager.egid = group 
+                    Puppet::SUIDManager.gid = group 
                 rescue => detail
                     Puppet.warning "could not change to group %s: %s" %
                         [group.inspect, detail]
@@ -128,10 +51,10 @@ module Util
             unless user
                 raise Puppet::Error, "No such user %s" % Puppet[:user]
             end
-            unless Process.uid == user
+            unless Puppet::SUIDManager.uid == user
                 begin
-                    Process.uid = user 
-                    Process.euid = user 
+                    Puppet::SUIDManager.uid = user 
+                    Puppet::SUIDManager.euid = user 
                 rescue
                     $stderr.puts "could not change to user %s" % user
                     exit(74)
@@ -221,6 +144,13 @@ module Util
     # Get the UID of a given user, whether a UID or name is provided
     def self.uid(user)
         uid = nil
+
+        # if we don't have any user info, warn and GTFO.
+        if !user
+            Puppet.warning "Username provided for lookup is nil" 
+            return nil
+        end
+
         if user =~ /^\d+$/
             user = Integer(user)
         end