puppetlabs / puppetlabs-firewall

Puppet Firewall Module
http://forge.puppetlabs.com/puppetlabs/firewall
Apache License 2.0
269 stars 455 forks source link

Non idempotent logs for empty firewall chains #1217

Open oliparcol opened 2 months ago

oliparcol commented 2 months ago

Describe the Bug

When applying on a server without any iptables rule the following puppet code with the resource firewallchain declared without any rule:

firewallchain { "FORWARD:mangle:IPv4":
    ensure => present,
    purge  => true,
}

The following output is always emitted:

Notice: /Stage[main]/Base::Firewall/Firewallchain[FORWARD:mangle:IPv4]/ensure: defined 'ensure' as 'present'
Notice: firewallchain[FORWARD:mangle:IPv4]: Updating: Finished in 0.000061 seconds

Expected Behavior

I would expect no output to be emitted.

Environment

Additional Context

I believe that the issue comes from the fact that iptables-save doesn't show empty tables. The code is therefore not able to distinguish an existing empty table from a non-existing one. Specifying the table with the -t option (e.g. iptables-save -t <table> does display the empty table).

2fa commented 2 months ago

1206 should fix that

corporate-gadfly commented 1 month ago

@2fa Would you mind looking at a comment in #1188 to see if it is related? Thanks in advance, for your time and attention.

2fa commented 1 month ago

@corporate-gadfly your reproduction steps looks very similar to this problem so i would assume that it is related, yes. They've merged it an hour ago so it should be fixed in the next version.

corporate-gadfly commented 1 month ago

No luck with 8.0.2.

Running:

puppet apply -e 'firewallchain {"PREROUTING:mangle:IPv4": ensure=>"present"}'

continues to give the output:

Notice: /Stage[main]/Main/Firewallchain[PREROUTING:mangle:IPv4]/ensure: defined 'ensure' as 'present'
Notice: firewallchain[PREROUTING:mangle:IPv4]: Creating: Creating Chain 'PREROUTING:mangle:IPv4' with {:name=>"PREROUTING:mangle:IPv4", :ensure=>"present", :purge=>false, :ignore_foreign=>false, :chain=>"PREROUTING", :table=>"mangle", :protocol=>"IPv4", :policy=>"accept"}
Notice: firewallchain[PREROUTING:mangle:IPv4]: Creating: Ensuring changes to 'PREROUTING:mangle:IPv4' persist
Notice: firewallchain[PREROUTING:mangle:IPv4]: Creating: Finished in 0.131559 seconds

Kindly let me know, if I can provide more details.

2fa commented 1 month ago

@corporate-gadfly do you have rules in table before that that contains * symbol anywhere?

You can check iptables-save output to be sure. And also you can check if that chains is already there.

corporate-gadfly commented 1 month ago

@corporate-gadfly do you have rules in table before that that contains * symbol anywhere?

No.

# iptables-save | grep '*'
*filter

You can check iptables-save output to be sure. And also you can check if that chains is already there.

# iptables-save -t mangle
# Generated by iptables-save v1.8.7 on Fri May 24 13:56:37 2024
*mangle
:PREROUTING ACCEPT [0:0]
:INPUT ACCEPT [0:0]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [0:0]
:POSTROUTING ACCEPT [0:0]
COMMIT
# Completed on Fri May 24 13:56:37 2024
2fa commented 1 month ago

@corporate-gadfly Looks like my fix works in a non nf_tables version of iptables. iptables-save doesn't output empty tables at all in a new version. Great stuff 😃

I will reopen original issue #1206

corporate-gadfly commented 1 month ago

TY:

Operating System: Ubuntu 22.04.4 LTS              
          Kernel: Linux 5.15.0-100-generic
    Architecture: x86-64
 Hardware Vendor: VMware, Inc.
  Hardware Model: VMware7,1

and:

# iptables -V
iptables v1.8.7 (nf_tables)