(MODULES-9578) Create authorized_key in root path

Previously, when the `target` property was set, the ssh_authorized_key
resource could not create directories/files within root-owned paths.
This behavior is due to the module switching context to the user, then
attempting to create the directory/file as the specified user,
ultimately failing because of insufficient permissions.

This commit adds a new parameter, `drop_privileges` which when set to
false allows the module to write a ssh_authorized_key file in a
privileged path. Due to the possible security implications of this,
the parameter must be manually specified in order to activate this
functionality.

A path is considered to be privileged/trusted if all of its ancestors:
- do not contain any symlinks
- have the same owner as the user who runs Puppet
- are not world/group writable

2 files changed, 56 insertions, 10 deletions

diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb
index 02a19eb..b10066e 100644
--- a/lib/puppet/provider/ssh_authorized_key/parsed.rb
+++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb
@@ -42,9 +42,28 @@ Puppet::Type.type(:ssh_authorized_key).provide(
     0o600
   end
 
-  def user
-    uid = Puppet::FileSystem.stat(target).uid
-    Etc.getpwuid(uid).name
+  def group_writable_perm
+    0o020
+  end
+
+  def group_writable?(path)
+    path.stat.mode & group_writable_perm != 0
+  end
+
+  def trusted_path
+    # return if the parent directory does not exist
+    return false unless Puppet::FileSystem.dir_exist?(target)
+    path = Puppet::FileSystem.pathname(target).dirname
+    until path.dirname.root?
+      path = path.realpath if path.symlink?
+      # do not trust if path is world or group writable
+      if path.stat.uid != Process.euid || path.world_writable? || group_writable?(path)
+        Puppet.debug('Path untrusted, will attempt to write as the target user')
+        return false
+      end
+      path = path.dirname
+    end
+    Puppet.debug('Path trusted, writing the file as the current user')
   end
 
   def flush
@@ -56,15 +75,30 @@ Puppet::Type.type(:ssh_authorized_key).provide(
     # so calling it here suppresses the later attempt by our superclass's flush method.
     self.class.backup_target(target)
 
-    Puppet::Util::SUIDManager.asuser(@resource.should(:user)) do
-      unless Puppet::FileSystem.exist?(dir = File.dirname(target))
-        Puppet.debug "Creating #{dir} as #{@resource.should(:user)}"
-        Dir.mkdir(dir, dir_perm)
-      end
+    # attempt to create the file as the specified user if we're not dropping privileges
+    if @resource[:drop_privileges]
+      Puppet::Util::SUIDManager.asuser(@resource.should(:user)) do
+        unless Puppet::FileSystem.exist?(dir = File.dirname(target))
+          Puppet.debug "Creating #{dir} as #{@resource.should(:user)}"
+          Dir.mkdir(dir, dir_perm)
+        end
+        super
 
+        File.chmod(file_perm, target)
+      end
+    # to avoid race conditions when handling permissions as a privileged user
+    # (CVE-2011-3870) we use the trusted_path method to ensure the entire
+    # directory structure is "safe" to write in
+    else
+      raise Puppet::Error, 'drop_privileges is false but the target path is not trusted' unless trusted_path
       super
 
-      File.chmod(file_perm, target)
+      uid = Puppet::Util.uid(@resource.should(:user))
+      gid = Puppet::Util.gid(@resource.should(:user))
+      File.open(target) do |target|
+        target.chown(uid, gid)
+        target.chmod(file_perm)
+      end
     end
   end
 
diff --git a/lib/puppet/type/ssh_authorized_key.rb b/lib/puppet/type/ssh_authorized_key.rb
index a36c069..648055c 100644
--- a/lib/puppet/type/ssh_authorized_key.rb
+++ b/lib/puppet/type/ssh_authorized_key.rb
@@ -1,3 +1,5 @@
+require 'puppet/parameter/boolean'
+
 module Puppet
   Type.newtype(:ssh_authorized_key) do
     @doc = "Manages SSH authorized keys. Currently only type 2 keys are supported.
@@ -48,6 +50,15 @@ module Puppet
       isnamevar
     end
 
+    newparam(:drop_privileges, boolean: true, parent: Puppet::Parameter::Boolean) do
+      desc "Whether to drop privileges when writing the key file. This is
+        useful for creating files in paths not writable by the target user. Note
+        the possible security implications of managing file ownership and
+        permissions as a privileged user."
+
+      defaultto true
+    end
+
     newproperty(:type) do
       desc 'The encryption type used.'
 
@@ -83,7 +94,8 @@ module Puppet
       desc "The absolute filename in which to store the SSH key. This
         property is optional and should be used only in cases where keys
         are stored in a non-standard location, for instance when not in
-        `~user/.ssh/authorized_keys`."
+        `~user/.ssh/authorized_keys`. The parent directory must be present
+        if the target is in a privileged path."
 
       defaultto :absent