Merge pull request #764 from opscode/ss/OC-7900

Remove ability to override selinux restorecon command.

7 files changed, 73 insertions, 87 deletions

diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index 04bf93536b..4f06aa9a63 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -324,19 +324,13 @@ class Chef
     windows_home_path = ENV['SYSTEMDRIVE'] + ENV['HOMEPATH'] if ENV['SYSTEMDRIVE'] && ENV['HOMEPATH']
     user_home(ENV['HOME'] || windows_home_path || ENV['USERPROFILE'])
 
-    # selinux command to restore file contexts
-    selinux_restorecon_command "/sbin/restorecon -R"
-    # guess if you're running selinux or not -- override this if it guesses wrong
-    selinux_enabled  Chef::Util::Selinux.new.selinuxenabled?
+    # Enable file permission fixup for selinux. Fixup will be done
+    # only if selinux is enabled in the system.
+    enable_selinux_file_permission_fixup true
 
     # for file resources, deploy files with either :move or :copy
     file_deploy_with :move
 
-    # for file resources, when they find a non-file (incl directories heirarchies) at their dest, set to
-    # true to have them default to obliterate whatever they find.
-    # BE CAREFUL: if this is true, file "/#{variable}" will result in: unlink "/" if variable.nil?
-    file_force_unlink false
-
     # do we create /tmp or %TEMP% files, or do we create temp files in the destination directory of the file?
     #  - on windows this avoids issues with permission inheritance with the %TEMP% directory (do not set this to false)
     #  - on unix this creates temp files like /etc/.sudoers.X-Y-Z and may create noise and make for itchy neckbeards
diff --git a/lib/chef/file_content_management/deploy/cp.rb b/lib/chef/file_content_management/deploy/cp.rb
index 98ee54119c..c6b1d6cd11 100644
--- a/lib/chef/file_content_management/deploy/cp.rb
+++ b/lib/chef/file_content_management/deploy/cp.rb
@@ -26,6 +26,12 @@
 class Chef
   class FileContentManagement
     class Deploy
+      #
+      # PURPOSE: This strategy preserves the inode, and will preserve modes + ownership
+      #          even if the user running chef cannot create that ownership (but has
+      #          rights to the file).  It is vulnerable to crashes in the middle of
+      #          writing the file which could result in corruption or zero-length files.
+      #
       class Cp
         def create(file)
           Chef::Log.debug("touching #{file} to create it")
diff --git a/lib/chef/file_content_management/deploy/mv_unix.rb b/lib/chef/file_content_management/deploy/mv_unix.rb
index 548571eb1b..9baaa9906d 100644
--- a/lib/chef/file_content_management/deploy/mv_unix.rb
+++ b/lib/chef/file_content_management/deploy/mv_unix.rb
@@ -16,17 +16,16 @@
 # limitations under the License.
 #
 
-#
-# PURPOSE: this strategy is atomic, and attempts to preserve file modes
-#
-# NOTE: there is no preserve flag to FileUtils.mv, and we want to preserve the dst file
-#       modes rather than the src file modes (preserve = true is what mv does already, we
-#       would like preserve = false which is tricky).
-#
-
 class Chef
   class FileContentManagement
     class Deploy
+      #
+      # PURPOSE: this strategy is atomic, and attempts to preserve file modes
+      #
+      # NOTE: there is no preserve flag to FileUtils.mv, and we want to preserve the dst file
+      #       modes rather than the src file modes (preserve = true is what mv does already, we
+      #       would like preserve = false which is tricky).
+      #
       class MvUnix
         def create(file)
           # this is very simple, but it ensures that ownership and file modes take
diff --git a/lib/chef/provider/directory.rb b/lib/chef/provider/directory.rb
index e4b4b42898..d256df4e11 100644
--- a/lib/chef/provider/directory.rb
+++ b/lib/chef/provider/directory.rb
@@ -102,7 +102,7 @@ class Chef
           end
         end
         do_acl_changes
-        do_selinux("-r")
+        do_selinux(true)
         load_resource_attributes_from_file(@new_resource)
       end
 
diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index 794e8208a6..06291a32a0 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -48,6 +48,7 @@ class Chef
       include Chef::Mixin::EnforceOwnershipAndPermissions
       include Chef::Mixin::Checksum
       include Chef::Mixin::ShellOut
+      include Chef::Util::Selinux
 
       extend Chef::Deprecation::Warnings
       include Chef::Deprecation::Provider::File
@@ -247,14 +248,18 @@ class Chef
         tempfile.unlink
       end
 
-      # this might be made into some kind of generic platform-dependent post-converge hook for
-      # file-like resources, but for now we only have the single selinux use case.
-      def do_selinux(command_args = nil)
-        if Chef::Config[:selinux_enabled] && resource_updated?
-          cmd = "#{Chef::Config[:selinux_restorecon_command]} #{command_args} #{@new_resource.path}"
-          converge_by("fix selinux context with #{cmd}") do
-            Chef::Log.debug("running #{cmd}")
-            shell_out!(cmd)
+      # This logic ideally will be  made into some kind of generic
+      # platform-dependent post-converge hook for file-like
+      # resources, but for now we only have the single selinux use
+      # case.
+      def do_selinux(recursive = false)
+        if resource_updated? && Chef::Config[:enable_selinux_file_permission_fixup]
+          if selinux_enabled?
+            converge_by("restore selinux security context") do
+              restore_security_context(@new_resource_path, recursive)
+            end
+          else
+            Chef::Log.debug "selinux utilities can not be found. Skipping selinux permission fixup."
           end
         end
       end
diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb
index 1bcf66259d..3d165017ff 100644
--- a/lib/chef/resource/file.rb
+++ b/lib/chef/resource/file.rb
@@ -48,7 +48,7 @@ class Chef
         @provider = Chef::Provider::File
         @binmode = Platform.windows? ? true : false
         @deploy_with = Chef::Config[:file_deploy_with]
-        @force_unlink = Chef::Config[:file_force_unlink]
+        @force_unlink = false
         @diff = nil
       end
 
diff --git a/lib/chef/util/selinux.rb b/lib/chef/util/selinux.rb
index 408ed4aa0a..b40a365901 100644
--- a/lib/chef/util/selinux.rb
+++ b/lib/chef/util/selinux.rb
@@ -22,70 +22,61 @@
 
 require 'chef/mixin/shell_out'
 
-#
-# NB: We take the approach that provisioning an selinux enabled server
-# without installing the selinux utilities is completely incoherent
-# and needs to be fixed in the provisioner / base image.
-#
-
 class Chef
   class Util
-    class Selinux
+    #
+    # IMPORTANT: We assume that selinux utilities are installed on an
+    # selinux enabled server. Provisioning an selinux enabled server
+    # without selinux utilities is not supported.
+    #
+    module Selinux
 
       include Chef::Mixin::ShellOut
 
-      attr_accessor :setenforce_path
-      attr_accessor :getenforce_path
-      attr_accessor :selinuxenabled_path
+      # We want to initialize below variables once during a
+      # chef-client run therefore they are class variables.
+      @@selinux_enabled = nil
+      @@restorecon_path = nil
+      @@selinuxenabled_path = nil
 
-      def setenforce_path
-        @setenforce_path ||= which("setenforce")
+      def selinux_enabled?
+        @@selinux_enabled = check_selinux_enabled? if @@selinux_enabled.nil?
+        @@selinux_enabled
       end
 
-      def getenforce_path
-        @getenforce_path ||= which("getenforce")
+      def restore_security_context(file_path, recursive = false)
+        if restorecon_path
+          restorecon_command = recursive ? "#{restorecon_path} -R -r" : "#{restorecon_path} -R"
+          restorecon_command += " #{file_path}"
+          Chef::Log.debug("Restoring selinux security content with #{restorecon_command}")
+          shell_out!(restorecon_command)
+        else
+          Chef::Log.warn "Can not find 'restorecon' on the system. Skipping selinux security context restore."
+        end
       end
 
-      def selinuxenabled_path
-        @selinuxenabled_path ||= which("selinuxenabled")
+      private
+
+      def restorecon_path
+        @@restorecon_path = which("restorecon") if @@restorecon_path.nil?
+        @@restorecon_path
       end
 
-      def setenforce(state)
-        if setenforce_path
-          case state
-          when :enforcing
-            shell_out!("#{setenforce_path} 1")
-          when :permissive
-            shell_out!("#{setenforce_path} 0")
-          else
-            raise ArgumentError, "Bad argument to Chef::Util::Seliux#setenforce: #{state}"
-          end
-        else
-          # FIXME?: manually roll our own setenforce
-        end
-        raise RuntimeError, "Called setenforce but binary does not exist (try installing selinux-utils or libselinux-utils)"
+      def selinuxenabled_path
+        @@selinuxenabled_path = which("selinuxenabled") if @@selinuxenabled_path.nil?
+        @@selinuxenabled_path
       end
 
-      def getenforce
-        if getenforce_path
-          cmd = shell_out!(getenforce_path)
-          case cmd.stdout
-          when /Permissive/i
-            return :permissive
-          when /Enforcing/i
-            return :enforcing
-          when /Disabled/i
-            return :disabled
-          else
-            raise RuntimeError, "Unknown output from getenforce: #{cmd.stdout}"
-          end
-        else
-          # FIXME?: manually roll our own getenforce
+      def which(cmd)
+        paths = ENV['PATH'].split(File::PATH_SEPARATOR) + [ '/bin', '/usr/bin', '/sbin', '/usr/sbin' ]
+        paths.each do |path|
+          filename = File.join(path, cmd)
+          return filename if File.executable?(filename)
         end
-        raise RuntimeError, "Called getenforce but binary does not exist (try installing selinux-utils or libselinux-utils)"
+        false
       end
 
-      def selinuxenabled?
+      def check_selinux_enabled?
         if selinuxenabled_path
           cmd = shell_out(selinuxenabled_path)
           case cmd.exitstatus
@@ -94,24 +85,15 @@ class Chef
           when 0
             return true
           else
-            raise RuntimeError, "Unknown exit code from selinuxenabled: #{cmd.exitstatus}"
+            raise RuntimeError, "Unknown exit code from command #{selinuxenabled_path}: #{cmd.exitstatus}"
           end
         else
-          # FIXME?: manually roll our own selinuxenabled
+          # We assume selinux is not enabled if selinux utils are not
+          # installed.
+          return false
         end
-        false
       end
 
-      private
-
-      def which(cmd)
-        paths = ENV['PATH'].split(File::PATH_SEPARATOR) + [ '/bin', '/usr/bin', '/sbin', '/usr/sbin' ]
-        paths.each do |path|
-          filename = File.join(path, cmd)
-          return filename if File.executable?(filename)
-        end
-        false
-      end
     end
   end
 end