From 7a669d49ac882cd82038a0ab9196f5800a9e91a1 Mon Sep 17 00:00:00 2001 From: Felipe Rodriguez Cortes Date: Mon, 20 Oct 2025 13:47:51 +0200 Subject: [PATCH] Perform selabel lookup during file resource synchronization The default values of the SELinux-related properties of a file resource are calculated at the beginning of the catalog application phase, before the catalog resources are actually synchronized. However, by the time the file resource is synchronized, its default value might be outdated due to new policy rules added in previous parts of the catalog (for example, a package was installed that included new policy rules) leading to the file being created with the wrong SELinux context. This commit delays the lookup of the default values until the very moment the file resource is being synchronized. More precisely, the following process is implemented: - A `:lookup` symbol is introduced as the default value. It means that the actual value is to be looked up later during synchronization. However, maintaining the previous behaviour, a default value of `nil` is used if either the platform is Windows or the user explicitly stated to ignore the default values (using the `selinux_ignore_defaults` parameter). - During resource synchronization, when the `insync?` method is executed, the `:lookup` symbol is replaced with the actual looked up value and then the sync status is calculated as usual. The unit test have been adjusted accordingly. --- lib/puppet/type/file/selcontext.rb | 20 ++++++++------- lib/puppet/util/selinux.rb | 4 +++ spec/unit/type/file/selinux_spec.rb | 39 ++++++++++++++++------------- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/lib/puppet/type/file/selcontext.rb b/lib/puppet/type/file/selcontext.rb index 8571d9ad2e..dec27b6687 100644 --- a/lib/puppet/type/file/selcontext.rb +++ b/lib/puppet/type/file/selcontext.rb @@ -40,11 +40,6 @@ def retrieve end def retrieve_default_context(property) - return nil if Puppet::Util::Platform.windows? - if @resource[:selinux_ignore_defaults] == :true - return nil - end - context = get_selinux_default_context_with_handle(@resource[:path], provider.class.selinux_handle, @resource[:ensure]) unless context return nil @@ -63,10 +58,17 @@ def insync?(value) debug("SELinux not available for this filesystem. Ignoring parameter.") true else + @should = @should.collect { |v| v == :lookup ? retrieve_default_context(name) : v } super end end + def validate(value) + unless value.is_a?(String) || value == :lookup + raise Puppet::Error, "The property #{name} must be either a string or :lookup" + end + end + def unsafe_munge(should) unless selinux_support? return should @@ -104,7 +106,7 @@ def sync enabled." @event = :file_changed - defaultto { retrieve_default_context(:seluser) } + defaultto { Puppet::Util::Platform.windows? || @resource[:selinux_ignore_defaults] == :true ? nil : :lookup } end Puppet::Type.type(:file).newproperty(:selrole, :parent => Puppet::SELFileContext) do @@ -115,7 +117,7 @@ def sync enabled." @event = :file_changed - defaultto { retrieve_default_context(:selrole) } + defaultto { Puppet::Util::Platform.windows? || @resource[:selinux_ignore_defaults] == :true ? nil : :lookup } end Puppet::Type.type(:file).newproperty(:seltype, :parent => Puppet::SELFileContext) do @@ -126,7 +128,7 @@ def sync enabled." @event = :file_changed - defaultto { retrieve_default_context(:seltype) } + defaultto { Puppet::Util::Platform.windows? || @resource[:selinux_ignore_defaults] == :true ? nil : :lookup } end Puppet::Type.type(:file).newproperty(:selrange, :parent => Puppet::SELFileContext) do @@ -138,6 +140,6 @@ def sync Security)." @event = :file_changed - defaultto { retrieve_default_context(:selrange) } + defaultto { Puppet::Util::Platform.windows? || @resource[:selinux_ignore_defaults] == :true ? nil : :lookup } end end diff --git a/lib/puppet/util/selinux.rb b/lib/puppet/util/selinux.rb index a73597ea65..303a81f962 100644 --- a/lib/puppet/util/selinux.rb +++ b/lib/puppet/util/selinux.rb @@ -188,6 +188,10 @@ def selinux_category_to_label(category) # We don't cache this, but there's already a ton of duplicate work # in the selinux handling code. + if category == :lookup + return category + end + path = Selinux.selinux_translations_path begin File.open(path).each do |line| diff --git a/spec/unit/type/file/selinux_spec.rb b/spec/unit/type/file/selinux_spec.rb index 11a4878f6f..96a2e26218 100644 --- a/spec/unit/type/file/selinux_spec.rb +++ b/spec/unit/type/file/selinux_spec.rb @@ -49,34 +49,39 @@ expect(@sel.retrieve).to eq(expectedresult) end - 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, :file).and_return(nil) + it "has a default value of :lookup by default" do + expect(@sel.default).to eq(:lookup) + end + + it "has a default value of nil on Windows", if: Puppet::Util::Platform.windows? do 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) + it "has a default value of nil if selinux_ignore_defaults is true" do + @resource[:selinux_ignore_defaults] = :true + expect(@sel.default).to be_nil + end + + it "looks up the default context when checking sync status", unless: Puppet::Util::Platform.windows? do + allow(@sel).to receive(:selinux_support?).and_return(true) + allow(@sel).to receive(:selinux_label_support?).with(@path).and_return(true) + + if param == :selrange + expect(@sel).to receive(:selinux_category_to_label).with(:lookup).and_return(:lookup) + end + @sel.should = [:lookup] + 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, :file).and_return("user_u:role_r:type_t:s0") - expectedresult = case param + + currentval = case param when :seluser; "user_u" when :selrole; "role_r" when :seltype; "type_t" when :selrange; "s0" end - expect(@sel.default).to eq(expectedresult) - end - - it "returns nil default context on Windows", if: Puppet::Util::Platform.windows? do - expect(@sel).to receive(:retrieve_default_context) - expect(@sel.default).to be_nil - end - - it "should return nil for defaults if selinux_ignore_defaults is true" do - @resource[:selinux_ignore_defaults] = :true - expect(@sel.default).to be_nil + expect(@sel.insync?(currentval)).to be(true) end it "should be able to set a new context" do