Module does not parse negated rules correctly about puppetlabs-firewall HOT 14 CLOSED

Yeah, I've been working on a new parser that should help with this: https://github.com/kbarber/ruby-iptables and https://rubygems.org/gems/iptables. It is, at least according to a lot of my tests - fairly much feature complete which is nice, as apposed to our current parser which is not. This is because I spent the extra effort analyzing the model that iptables uses and making the iptables.rb code as generic as possible to fit that model.

The idea is that the gem 'iptables' is a closer model to iptables and the iptables provider can then just act as a bridge. I haven't yet done this work though ... its in my grand plan, just wanted to get the iptables library stable first.

from puppetlabs-firewall.

is there a version I can safely revert to?

from puppetlabs-firewall.

This is a really critical bug. It is impossible to use firewall module if rules with "!" get inserted. Is it possible to workaround this bug or revert to some stable version of firewall module ?

from puppetlabs-firewall.

We (DTG) are staying on v0.0.4 rather than upgrading to 0.3.0 (there is probably a working version higher than our current one but I am not going looking for it).

from puppetlabs-firewall.

How's progress on this going kbarber? This is a big stinky problem for us too.

from puppetlabs-firewall.

Yep, hard to do the rules that IPTables automatically sets up for bridging interfaces which is needed for KVM without this feature.

from puppetlabs-firewall.

It seems this bugs has not been resolved. I have try the latest version but it failed.

from puppetlabs-firewall.

I'll just add my +1 here: this is messing me up. I can't even create the rule outside of puppet, because the provider blows up badly if I've got that rule with a negation in there.

from puppetlabs-firewall.

Since this bug, which is really quite major, has been here for 7 months, I wasn't inclined to wait for a new parser.

This will solve the problem enough to allow you to add other rules. It does NOT allow you to add your own negated rules, but it does not appear to munge or break any that are already there:

 [rlpowell@shell01 puppet3]$ gd modules/firewall/
 diff --git a/modules/firewall/lib/puppet/provider/firewall/iptables.rb b/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 index 1e8617e..3ed4541 100644
 --- a/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 +++ b/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 @@ -166,6 +166,9 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
      # so it behaves like --comment
      values = values.sub(/--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1 \2"')

 +    # Remove negators entirely
 +    values = values.gsub(%r{!},'')
 +
      # Trick the system for booleans
      known_booleans.each do |bool|
        if bool == :socket then

from puppetlabs-firewall.

Having done so, on some hosts I got this:

 Error: /Firewall[010 iptables PUPPET: accept all to lo interface]: Could not evaluate: Invalid address from IPAddr.new: eth0

Apparently caused by:

-A POSTROUTING -s 172.30.0.0/16 -o eth0 -j MASQUERADE --random

And, in fact, it's the --random that causes the problem, despite the message.

I fixed this with a hack just as nasty as the last, but again, it doesn't seem to break any of the currently extant rules, which is all I care about.

The new diff:

  diff --git a/modules/firewall/lib/puppet/provider/firewall/iptables.rb b/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 index 1e8617e..f43084d 100644
 --- a/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 +++ b/modules/firewall/lib/puppet/provider/firewall/iptables.rb
 @@ -166,6 +166,12 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
      # so it behaves like --comment
      values = values.sub(/--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1 \2"')

 +    # Remove negators entirely
 +    values = values.gsub(%r{!},'')
 +
 +    # Remove interface flags entirely
 +    values = values.gsub(%r{[-][-]random},'')
 +
      # Trick the system for booleans
      known_booleans.each do |bool|
        if bool == :socket then

from puppetlabs-firewall.

+1 on KVM

from puppetlabs-firewall.

Hello,
What's the status of this issue? is the proposed patch safe to use so far?

from puppetlabs-firewall.

I don't know that I would consider my updates a patch. They don't improve the capability of the module, they simply throw out the things that confuse it. Having said that, umm, it works fine for me! :D

from puppetlabs-firewall.

This bug would be fixed by PR #267 which both handles existing rules and lets you add them.

from puppetlabs-firewall.