(PUP-7559) Pass correct mode when lookup SELinux default context

Originally when the file did not exist we simply passed the
mode as zero causing the file type bits not to be set which
means matchpatchcon(3) won't lookup the type based on the
correct type.

This changes so that we set the file type bits in the mode_t
passed down to the SELinux libraries based on the resource
ensure set to present, file, directory or link on the File
resource.

This fixes [1] and can be observed for SELinux file contexts
that modifies files in top directory but only for symlinks for
example this [2] httpd file context change.

The patch is backward compatible and does not break the API
for the methods in the Puppet::Util::SELinux module.

[1] https://tickets.puppetlabs.com/browse/PUP-7559
[2] fedora-selinux/selinux-policy-contrib@43318bf

Author: tobias-urdin

lib/puppet/type/file/selcontext.rb

@@ -42,7 +42,7 @@ def retrieve_default_context(property)
         return nil
       end
 
-      context = self.get_selinux_default_context(@resource[:path])
+      context = self.get_selinux_default_context(@resource[:path], @resource[:ensure].to_s)
       unless context
         return nil
       end

lib/puppet/util/selinux.rb

@@ -13,6 +13,10 @@
 
 module Puppet::Util::SELinux
 
+  S_IFREG = 0100000
+  S_IFDIR = 0040000
+  S_IFLNK = 0120000
+
   def self.selinux_support?
     return false unless defined?(Selinux)
     if Selinux.is_selinux_enabled == 1
@@ -38,7 +42,7 @@ def get_selinux_current_context(file)
 
   # Retrieve and return the default context of the file.  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(file)
+  def get_selinux_default_context(file, 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 matchpathcon would return
@@ -48,8 +52,14 @@ def get_selinux_default_context(file)
     begin
       filestat = file_lstat(file)
       mode = filestat.mode
-    rescue Errno::EACCES, Errno::ENOENT
+    rescue Errno::EACCES
       mode = 0
+    rescue Errno::ENOENT
+      if resource_ensure
+        mode = get_create_mode(resource_ensure)
+      else
+        mode = 0
+      end
     end
 
     retval = Selinux.matchpathcon(file, mode)
@@ -136,8 +146,8 @@ def set_selinux_context(file, value, component = false)
   # Puppet uses.  This will set the file's SELinux context to the policy's
   # default context (if any) if it differs from the context currently on
   # the file.
-  def set_selinux_default_context(file)
-    new_context = get_selinux_default_context(file)
+  def set_selinux_default_context(file, resource_ensure=nil)
+    new_context = get_selinux_default_context(file, resource_ensure)
     return nil unless new_context
     cur_context = get_selinux_current_context(file)
     if new_context != cur_context
@@ -198,6 +208,23 @@ def selinux_label_support?(file)
     filesystems.include?(fstype)
   end
 
+  # Get mode file type bits set based on ensure on
+  # the file resource. This helps SELinux determine
+  # what context a new resource being created should have.
+  def get_create_mode(resource_ensure)
+    mode = 0
+    return mode if resource_ensure == 'absent'
+    case resource_ensure
+    when "present", "file"
+      mode = 0 | S_IFREG
+    when "directory"
+      mode = 0 | S_IFDIR
+    when "link"
+      mode = 0 | S_IFLNK
+    end
+    mode
+  end
+
   # Internal helper function to read and parse /proc/mounts
   def read_mounts
     mounts = ""

spec/unit/type/file/selinux_spec.rb

@@ -50,13 +50,13 @@
     end
 
     it "should handle no default gracefully" do
-      expect(@sel).to receive(:get_selinux_default_context).with(@path).and_return(nil)
+      expect(@sel).to receive(:get_selinux_default_context).with(@path, @resource[:ensure].to_s).and_return(nil)
       expect(@sel.default).to be_nil
     end
 
     it "should be able to detect matchpathcon defaults" do
       allow(@sel).to receive(:debug)
-      expect(@sel).to receive(:get_selinux_default_context).with(@path).and_return("user_u:role_r:type_t:s0")
+      expect(@sel).to receive(:get_selinux_default_context).with(@path, @resource[:ensure].to_s).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

@@ -159,7 +159,7 @@
       end
     end
 
-    it "handles no such file or directory errors by issuing a warning" do
+    it "backward compatibly handles no such file or directory errors by issuing a warning when resource_ensure not set" do
       without_partial_double_verification do
         allow(self).to receive(:selinux_support?).and_return(true)
         allow(self).to receive(:selinux_label_support?).and_return(true)
@@ -170,6 +170,51 @@
       end
     end
 
+    it "should determine mode based on resource ensure when set to file" do
+      without_partial_double_verification do
+        allow(self).to receive(:selinux_support?).and_return(true)
+        allow(self).to receive(:selinux_label_support?).and_return(true)
+        allow(Selinux).to receive(:matchpathcon).with("/root/chuj", 32768).and_return(-1)
+        allow(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context("/root/chuj", "present")).to be_nil
+        expect(get_selinux_default_context("/root/chuj", "file")).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to dir" do
+      without_partial_double_verification do
+        allow(self).to receive(:selinux_support?).and_return(true)
+        allow(self).to receive(:selinux_label_support?).and_return(true)
+        allow(Selinux).to receive(:matchpathcon).with("/root/chuj", 16384).and_return(-1)
+        allow(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context("/root/chuj", "directory")).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to link" do
+      without_partial_double_verification do
+        allow(self).to receive(:selinux_support?).and_return(true)
+        allow(self).to receive(:selinux_label_support?).and_return(true)
+        allow(Selinux).to receive(:matchpathcon).with("/root/chuj", 40960).and_return(-1)
+        allow(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context("/root/chuj", "link")).to be_nil
+      end
+    end
+
+    it "should determine mode based on resource ensure when set to unknown" do
+      without_partial_double_verification do
+        allow(self).to receive(:selinux_support?).and_return(true)
+        allow(self).to receive(:selinux_label_support?).and_return(true)
+        allow(Selinux).to receive(:matchpathcon).with("/root/chuj", 0).and_return(-1)
+        allow(self).to receive(:file_lstat).with("/root/chuj").and_raise(Errno::ENOENT, "/root/chuj")
+
+        expect(get_selinux_default_context("/root/chuj", "unknown")).to be_nil
+      end
+    end
+
     it "should return nil if matchpathcon returns failure" do
       without_partial_double_verification do
         expect(self).to receive(:selinux_support?).and_return(true)
@@ -329,21 +374,44 @@
     end
 
     it "should return nil if no default context exists" do
-      expect(self).to receive(:get_selinux_default_context).with("/foo").and_return(nil)
+      expect(self).to receive(:get_selinux_default_context).with("/foo", nil).and_return(nil)
       expect(set_selinux_default_context("/foo")).to be_nil
     end
 
     it "should do nothing and return nil if the current context matches the default context" do
-      expect(self).to receive(:get_selinux_default_context).with("/foo").and_return("user_u:role_r:type_t")
+      expect(self).to receive(:get_selinux_default_context).with("/foo", nil).and_return("user_u:role_r:type_t")
       expect(self).to receive(:get_selinux_current_context).with("/foo").and_return("user_u:role_r:type_t")
       expect(set_selinux_default_context("/foo")).to be_nil
     end
 
     it "should set and return the default context if current and default do not match" do
-      expect(self).to receive(:get_selinux_default_context).with("/foo").and_return("user_u:role_r:type_t")
+      expect(self).to receive(:get_selinux_default_context).with("/foo", nil).and_return("user_u:role_r:type_t")
       expect(self).to receive(:get_selinux_current_context).with("/foo").and_return("olduser_u:role_r:type_t")
       expect(self).to receive(:set_selinux_context).with("/foo", "user_u:role_r:type_t").and_return(true)
       expect(set_selinux_default_context("/foo")).to eq("user_u:role_r:type_t")
     end
   end
+
+  describe "get_create_mode" do
+    it "should return 0 if the resource is absent" do
+      expect(get_create_mode("absent")).to eq(0)
+    end
+
+    it "should return mode with file type set to S_IFREG when resource is file" do
+      expect(get_create_mode("present")).to eq(32768)
+      expect(get_create_mode("file")).to eq(32768)
+    end
+
+    it "should return mode with file type set to S_IFDIR when resource is dir" do
+      expect(get_create_mode("directory")).to eq(16384)
+    end
+
+    it "should return mode with file type set to S_IFLNK when resource is link" do
+      expect(get_create_mode("link")).to eq(40960)
+    end
+
+    it "should return 0 for everything else" do
+      expect(get_create_mode("unknown")).to eq(0)
+    end
+  end
 end