Module does not parse negated rules correctly

[asciipip]

I have an iptables configuration that contains a few rules in the nat table similar to the following:

-A POSTROUTING -s 192.168.1.0/24 ! -d 192.168.1.0/24 -p tcp -j MASQUERADE --to-ports 1024-65535

The parsing code appears to be choking on the exclamation point in the rule:

$ puppet agent --test --debug
[snip]
debug: Prefetching iptables resources for firewall
debug: Puppet::Type::Firewall::ProviderIptables: [prefetch(resources)]
debug: Puppet::Type::Firewall::ProviderIptables: [instances]
debug: Puppet::Type::Firewall::ProviderIptables: Executing '/sbin/iptables-save'
err: Could not prefetch firewall provider 'iptables': Invalid address from IPAddr.new: !
[snip]
info: Applying configuration version '1363803270'
debug: Puppet::Type::Firewall::ProviderIptables: [instances]
debug: Puppet::Type::Firewall::ProviderIptables: Executing '/sbin/iptables-save'
err: /Firewall[500 accept munin connections from munin master]: Could not evaluate: Invalid address from IPAddr.new: !

I would maybe like it not to do that or at least fail in a way that still allows it to apply the rules I've specified in my manifests.

[kbarber]

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.

[bodepd]

is there a version I can safely revert to?

[aglarendil]

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 ?

[drt24]

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).

[mrwacky42]

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

[ITBlogger]

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

[NewpTone]

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

[BillWeiss]

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.

[rlpowell]

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

[rlpowell]

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

[cardil]

+1 on KVM

[luisfdez]

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

[rlpowell]

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

[phemmer]

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