Merge pull request puppetlabs#9448 from joshcooper/fix-file-resource-seltype-9431

Add file mode-awareness to selabel_lookup

Author: joshcooper

lib/puppet/type/file/selcontext.rb

@@ -45,7 +45,7 @@ def retrieve_default_context(property)
         return nil
       end
 
-      context = get_selinux_default_context_with_handle(@resource[:path], provider.class.selinux_handle)
+      context = get_selinux_default_context_with_handle(@resource[:path], provider.class.selinux_handle, @resource[:ensure])
       unless context
         return nil
       end

lib/puppet/util/selinux.rb

@@ -55,24 +55,16 @@ def get_selinux_default_context(file, resource_ensure = nil)
 
     # If the file exists we should pass the mode to matchpathcon for the most specific
     # matching.  If not, we can pass a mode of 0.
-    begin
-      filestat = file_lstat(file)
-      mode = filestat.mode
-    rescue Errno::EACCES
-      mode = 0
-    rescue Errno::ENOENT
-      if resource_ensure
-        mode = get_create_mode(resource_ensure)
-      else
-        mode = 0
-      end
-    end
+    mode = file_mode(file, resource_ensure)
 
     retval = Selinux.matchpathcon(file, mode)
     retval == -1 ? nil : retval[1]
   end
 
-  def get_selinux_default_context_with_handle(file, handle)
+  # Retrieve and return the default context of the file using an selinux handle.
+  # If we don't have SELinux support or if the SELinux call fails to file a
+  # default then return nil.
+  def get_selinux_default_context_with_handle(file, handle, resource_ensure = nil)
     return nil unless selinux_support?
     # If the filesystem has no support for SELinux labels, return a default of nil
     # instead of what selabel_lookup would return
@@ -81,7 +73,11 @@ def get_selinux_default_context_with_handle(file, handle)
     # Handle is needed for selabel_lookup
     raise ArgumentError, _("Cannot get default context with nil handle") unless handle
 
-    retval = Selinux.selabel_lookup(handle, file, 0)
+    # If the file exists we should pass the mode to selabel_lookup for the most specific
+    # matching.  If not, we can pass a mode of 0.
+    mode = file_mode(file, resource_ensure)
+
+    retval = Selinux.selabel_lookup(handle, file, mode)
     retval == -1 ? nil : retval[1]
   end
 
@@ -245,6 +241,22 @@ def get_create_mode(resource_ensure)
     mode
   end
 
+  # If the file/directory/symlink exists, return its mode. Otherwise, get the default mode
+  # that should be used to create the file/directory/symlink taking into account the desired
+  # file type specified in +resource_ensure+.
+  def file_mode(file, resource_ensure)
+    filestat = file_lstat(file)
+    filestat.mode
+  rescue Errno::EACCES
+    0
+  rescue Errno::ENOENT
+    if resource_ensure
+      get_create_mode(resource_ensure)
+    else
+      0
+    end
+  end
+
   # Internal helper function to read and parse /proc/mounts
   def read_mounts
     mounts = ''.dup

spec/unit/type/file/selinux_spec.rb

@@ -51,15 +51,15 @@
 
     it "should handle no default gracefully" do
       skip if Puppet::Util::Platform.windows?
-      expect(@sel).to receive(:get_selinux_default_context_with_handle).with(@path, nil).and_return(nil)
+      expect(@sel).to receive(:get_selinux_default_context_with_handle).with(@path, nil, :file).and_return(nil)
       expect(@sel.default).to be_nil
     end
 
     it "should be able to detect default context on platforms other than Windows", unless: Puppet::Util::Platform.windows? do
       allow(@sel).to receive(:debug)
       hnd = double("SWIG::TYPE_p_selabel_handle")
       allow(@sel.provider.class).to receive(:selinux_handle).and_return(hnd)
-      expect(@sel).to receive(:get_selinux_default_context_with_handle).with(@path, hnd).and_return("user_u:role_r:type_t:s0")
+      expect(@sel).to receive(:get_selinux_default_context_with_handle).with(@path, hnd, :file).and_return("user_u:role_r:type_t:s0")
       expectedresult = case param
         when :seluser; "user_u"
         when :selrole; "role_r"

spec/unit/util/selinux_spec.rb

@@ -6,7 +6,7 @@
 describe Puppet::Util::SELinux do
   include Puppet::Util::SELinux
 
-  let(:selinux) { double('selinux', is_selinux_enabled: false) }
+  let(:selinux) { double('selinux', is_selinux_enabled: 0) }
 
   before :each do
     stub_const('Selinux', selinux)
@@ -252,6 +252,93 @@
       end
     end
 
+    it "should return nil when permission denied errors are encountered" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true)
+        expect(self).to receive(:selinux_label_support?).and_return(true)
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd, "/root/chuj", 0).and_return(-1)
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::EACCES, "/root/chuj")
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd)).to be_nil
+      end
+    end
+
+    it "should return nil when no such file or directory errors are encountered and resource_ensure is unset" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true)
+        expect(self).to receive(:selinux_label_support?).and_return(true)
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd, "/root/chuj", 0).and_return(-1)
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd)).to be_nil
+      end
+    end
+
+    it "should pass through lstat mode when file exists" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true).twice
+        expect(self).to receive(:selinux_label_support?).and_return(true).twice
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        fstat = double("File::Stat", :mode => 16384)
+        expect(Selinux).to receive(:selabel_lookup).with(hnd,  "/root/chuj", fstat.mode).and_return([0, "user_u:role_r:type_t:s0"]).twice
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_return(fstat).twice
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd)).to eq("user_u:role_r:type_t:s0")
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, :file)).to eq("user_u:role_r:type_t:s0")
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to file" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true).twice
+        expect(self).to receive(:selinux_label_support?).and_return(true).twice
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd,  "/root/chuj", 32768).and_return(-1).twice
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj").twice
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, :present)).to be_nil
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, :file)).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to dir" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true)
+        expect(self).to receive(:selinux_label_support?).and_return(true)
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd, "/root/chuj", 16384).and_return(-1)
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, :directory)).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to link" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true)
+        expect(self).to receive(:selinux_label_support?).and_return(true)
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd, "/root/chuj", 40960).and_return(-1)
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, :link)).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to unknown" do
+      without_partial_double_verification do
+        expect(self).to receive(:selinux_support?).and_return(true)
+        expect(self).to receive(:selinux_label_support?).and_return(true)
+        hnd = double("SWIG::TYPE_p_selabel_handle")
+        expect(Selinux).to receive(:selabel_lookup).with(hnd, "/root/chuj", 0).and_return(-1)
+        expect(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context_with_handle("/root/chuj", hnd, "unknown")).to be_nil
+      end
+    end
+
     it "should raise an ArgumentError when handle is nil" do
       allow(self).to receive(:selinux_support?).and_return(true)
       allow(self).to receive(:selinux_label_support?).and_return(true)