From 86620cff165fbffe623e0c8c7bde1204ee76bf30 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Thu, 8 Jan 2015 18:06:10 +0100 Subject: [PATCH 1/2] Lookup username from uuid When using the uid feature of the firewall module, it did not work with string based usernames as documented. The uid propery always synchronized with a message of does not match . This code overrides the uid getter method to perform a check of both the data from the property hash as well as using that data (assuming it is a uid) to resolve the username. While this patch is pretty simple, I have only tested it on Ubuntu 14.04. I am not sure if it could be problematic with other versions. I have not yet written tests b/c I wanted to submit my proposed fix for discussion while I get those written. --- lib/puppet/type/firewall.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb index e7cb04b71..88d1aaf53 100644 --- a/lib/puppet/type/firewall.rb +++ b/lib/puppet/type/firewall.rb @@ -730,6 +730,11 @@ def should_to_s(value) only, as iptables does not accept multiple uid in a single statement. EOS + def insync?(is) + require 'etc' + return is.to_s == @should.first.to_s || Etc.getpwuid(Integer(is)).name == @should.first.to_s + end + end newproperty(:gid, :required_features => :owner) do From 19ea03665630ea7e9719b4610055c66d807bdd7f Mon Sep 17 00:00:00 2001 From: Jonathan Tripathy Date: Wed, 21 Jan 2015 00:48:12 -0800 Subject: [PATCH 2/2] Fix for MODULES-1688 Re-applying a manifest with an unchanged UID will now not re-apply the rule unnecessarily. --- lib/puppet/type/firewall.rb | 20 ++++- spec/acceptance/firewall_uid_spec.rb | 117 +++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 spec/acceptance/firewall_uid_spec.rb diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb index 88d1aaf53..b2bccb96d 100644 --- a/lib/puppet/type/firewall.rb +++ b/lib/puppet/type/firewall.rb @@ -732,7 +732,25 @@ def should_to_s(value) EOS def insync?(is) require 'etc' - return is.to_s == @should.first.to_s || Etc.getpwuid(Integer(is)).name == @should.first.to_s + + # The following code allow us to take into consideration unix mappings + # between string usernames and UIDs (integers). We also need to ignore + # spaces as they are irrelevant with respect to rule sync. + + is = is.gsub(/\s+/,'') + + if is.start_with?('!') + lookup_id = is.gsub(/^!/,'') + negate = '!' + else + lookup_id = is + negate = '' + end + + resolve = Etc.getpwuid(Integer(lookup_id)).name + resolve_with_negate = "#{negate}#{resolve}" + + return is.to_s == @should.first.to_s.gsub(/\s+/,'') || resolve_with_negate == @should.first.to_s.gsub(/\s+/,'') end end diff --git a/spec/acceptance/firewall_uid_spec.rb b/spec/acceptance/firewall_uid_spec.rb new file mode 100644 index 000000000..50728b45b --- /dev/null +++ b/spec/acceptance/firewall_uid_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper_acceptance' + +describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do + + describe 'reset' do + it 'deletes all rules' do + shell('iptables --flush; iptables -t nat --flush; iptables -t mangle --flush') + end + it 'deletes all ip6tables rules' do + shell('ip6tables --flush; ip6tables -t nat --flush; ip6tables -t mangle --flush') + end + end + + describe "uid tests" do + context 'uid set to root' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '801 - test': + chain => 'OUTPUT', + action => accept, + uid => 'root', + proto => 'all', + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -m owner --uid-owner (0|root) -m comment --comment "801 - test" -j ACCEPT/) + end + end + end + + context 'uid set to !root' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '802 - test': + chain => 'OUTPUT', + action => accept, + uid => '!root', + proto => 'all', + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -m owner ! --uid-owner (0|root) -m comment --comment "802 - test" -j ACCEPT/) + end + end + end + + context 'uid set to 0' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '803 - test': + chain => 'OUTPUT', + action => accept, + uid => '0', + proto => 'all', + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -m owner --uid-owner (0|root) -m comment --comment "803 - test" -j ACCEPT/) + end + end + end + + context 'uid set to !0' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '804 - test': + chain => 'OUTPUT', + action => accept, + uid => '!0', + proto => 'all', + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -m owner ! --uid-owner (0|root) -m comment --comment "804 - test" -j ACCEPT/) + end + end + end + + end + +end