Hi,
The manifest folder, containing what users will actually call, hasn't
change a bit since version 8.0.0-3. What changed is the lib folder, for
which I attached a diff, which is the only relevant changes.
The important bits that we need is:
require_relative
in all the imports under the lib/puppet_x folder. But I do support also
all the other changes that are making the module much better, without
changing any of its functionality.
Also, more generally, this version is to be used the current
puppetserver when the older one wasn't patched / fixed for it.
Cheers,
Thomas Goirand (zigo)
diff -u -N -r ../lib/facter/iptables_persistent_version.rb lib/facter/iptables_persistent_version.rb
--- ../lib/facter/iptables_persistent_version.rb 2025-07-22 10:14:58.262456144 +0200
+++ lib/facter/iptables_persistent_version.rb 2025-07-22 10:15:08.710721654 +0200
@@ -1,7 +1,7 @@
# frozen_string_literal: true
Facter.add(:iptables_persistent_version) do
- confine operatingsystem: ['Debian', 'Ubuntu']
+ confine 'os.name': ['Debian', 'Ubuntu']
setcode do
# Throw away STDERR because dpkg >= 1.16.7 will make some noise if the
# package isn't currently installed.
diff -u -N -r ../lib/puppet/provider/firewall/firewall.rb lib/puppet/provider/firewall/firewall.rb
--- ../lib/puppet/provider/firewall/firewall.rb 2025-07-22 10:14:58.262456144 +0200
+++ lib/puppet/provider/firewall/firewall.rb 2025-07-22 10:15:08.710721654 +0200
@@ -19,7 +19,7 @@
# Regex used to retrieve table name
$table_name_regex = %r{^\*(nat|mangle|filter|raw|rawpost|broute|security)}
# Regex used to retrieve Rules
- $rules_regex = %r{(-A.*)\n}
+ $rules_regex = %r{^(-A.*)\n}
# Base command
$base_command = {
'IPv4' => 'iptables -t',
@@ -325,8 +325,8 @@
def insync?(context, _name, property_name, is_hash, should_hash)
context.debug("Checking whether '#{property_name}' is out of sync")
- # If either value is nil, no custom logic is required
- return nil if is_hash[property_name].nil? || should_hash[property_name].nil?
+ # If either value is nil, no custom logic is required unless property is source or destination
+ return nil if (is_hash[property_name].nil? || should_hash[property_name].nil?) && ![:source, :destination].include?(property_name)
case property_name
when :protocol
@@ -376,9 +376,16 @@
end
# If 'is' or 'should' contain anything other than digits or digit range,
- # we assume that we have to do a lookup to convert to UID
- is = Etc.getpwnam(is).uid unless is[%r{[0-9]+(-[0-9]+)?}] == is
- should = Etc.getpwnam(should).uid unless should[%r{[0-9]+(-[0-9]+)?}] == should
+ # we assume that we have to do a lookup to convert to UID or GID
+ if property_name == :uid
+ is = Etc.getpwnam(is).uid unless is[%r{[0-9]+(-[0-9]+)?}] == is
+ should = Etc.getpwnam(should).uid unless should[%r{[0-9]+(-[0-9]+)?}] == should
+ end
+
+ if property_name == :gid
+ is = Etc.getgrnam(is).gid unless is[%r{[0-9]+(-[0-9]+)?}] == is
+ should = Etc.getgrnam(should).gid unless should[%r{[0-9]+(-[0-9]+)?}] == should
+ end
"#{is_negate}#{is}" == "#{should_negate}#{should}"
when :mac_source, :jump
@@ -394,16 +401,11 @@
is = PuppetX::Firewall::Utility.log_level_name_to_number(is_hash[property_name])
should = PuppetX::Firewall::Utility.log_level_name_to_number(should_hash[property_name])
is == should
- when :set_mark
+ when :set_mark, :match_mark, :connmark
# Ensure that the values are compared to eachother in hexidecimal format
is = PuppetX::Firewall::Utility.mark_mask_to_hex(is_hash[property_name])
should = PuppetX::Firewall::Utility.mark_mask_to_hex(should_hash[property_name])
is == should
- when :match_mark, :connmark
- # Ensure that the values are compared to eachother in hexidecimal format
- is = PuppetX::Firewall::Utility.mark_to_hex(is_hash[property_name])
- should = PuppetX::Firewall::Utility.mark_to_hex(should_hash[property_name])
- is == should
when :time_start, :time_stop
# Ensure the values are compared in full `00:00:00` format
is = is_hash[property_name]
@@ -416,28 +418,32 @@
when :dport, :sport, :state, :ctstate, :ctstatus
is = is_hash[property_name]
should = should_hash[property_name]
+ ports = [:dport, :sport]
- # Unique logic is only needed when both values are arrays
- return nil unless is.is_a?(Array) && should.is_a?(Array)
+ if is.is_a?(Array) && should.is_a?(Array)
+ # Ensure values are sorted
+ # Ensure any negation includes only the first value
+ is_negated = true if %r{^!\s}.match?(is[0].to_s)
+ is.each_with_index do |_value, _index|
+ is = is.map { |value| value.to_s.tr('! ', '') }.sort
+ end
+ is[0] = ['!', is[0]].join(' ') if is_negated
+
+ should_negated = true if %r{^!\s}.match?(should[0].to_s)
+ should.each_with_index do |_value, _index|
+ should = should.map { |value| value.to_s.tr('! ', '') }.sort
+ # Port range can be passed as `-` but will always be set/returned as `:`
+ should = should.map { |value| value.to_s.tr('-', ':') }.sort if ports.include?(property_name)
+ end
+ should[0] = ['!', should[0]].join(' ') if should_negated
- # Ensure values are sorted
- # Ensure any negation includes only the first value
- is_negated = true if %r{^!\s}.match?(is[0].to_s)
- is.each_with_index do |_value, _index|
- is = is.map { |value| value.to_s.tr('! ', '') }.sort
- end
- is[0] = ['!', is[0]].join(' ') if is_negated
-
- should_negated = true if %r{^!\s}.match?(should[0].to_s)
- should.each_with_index do |_value, _index|
- should = should.map { |value| value.to_s.tr('! ', '') }.sort
+ is == should
+ elsif is.is_a?(String) && should.is_a?(String)
# Port range can be passed as `-` but will always be set/returned as `:`
- ports = [:dport, :sport]
- should = should.map { |value| value.to_s.tr('-', ':') }.sort if ports.include?(property_name)
- end
- should[0] = ['!', should[0]].join(' ') if should_negated
+ should = should.tr('-', ':') if ports.include?(property_name)
- is == should
+ is == should
+ end
when :string_hex
# Compare the values with any whitespace removed
is = is_hash[property_name].to_s.gsub(%r{\s+}, '')
@@ -471,6 +477,9 @@
iptables_list.scan($table_regex).each do |table|
table_name = table[0].scan($table_name_regex)[0][0]
table[0].scan($rules_regex).each do |rule|
+ # iptables-save escapes ' symbol in it's output for some reason which leads to an incorrect command
+ # We need to manually replace \' to '
+ rule[0].gsub!("\\'", "'")
raw_rules = if basic
Puppet::Provider::Firewall::Firewall.rule_to_name(context, rule[0], table_name, protocol)
else
@@ -494,7 +503,7 @@
rule_hash[:table] = table_name
rule_hash[:protocol] = protocol
- name_regex = Regexp.new("#{$resource_map[:name]}\\s(?:\"([^\"]*)|([^\"\\s]*))")
+ name_regex = Regexp.new("#{$resource_map[:name]}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
name_value = rule.scan(name_regex)[0]
# Combine the returned values and remove and trailing or leading whitespace
rule_hash[:name] = [name_value[0], name_value[1]].join(' ').strip if name_value
@@ -532,7 +541,7 @@
# When only a single word comment is returned no quotes are given, so we must check for this as well
# First find if flag is present, add a space to ensure accuracy with the more simplistic flags; i.e. `-i`
if rule.match(Regexp.new("#{value}\\s"))
- value_regex = Regexp.new("(?:(!\\s))?#{value}\\s(?:\"([^\"]*)|([^\"\\s]*))")
+ value_regex = Regexp.new("(?:(!\\s))?#{value}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
key_value = rule.scan(value_regex)[0]
# Combine the returned values and remove and trailing or leading whitespace
key_value[1] = [key_value[0], key_value[1], key_value[2]].join
@@ -893,8 +902,8 @@
# `set_mark`, `match_mark` and `connmark` must be applied in hexidecimal format
should[:set_mark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:set_mark]) if should[:set_mark]
- should[:match_mark] = PuppetX::Firewall::Utility.mark_to_hex(should[:match_mark]) if should[:match_mark]
- should[:connmark] = PuppetX::Firewall::Utility.mark_to_hex(should[:connmark]) if should[:connmark]
+ should[:match_mark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:match_mark]) if should[:match_mark]
+ should[:connmark] = PuppetX::Firewall::Utility.mark_mask_to_hex(should[:connmark]) if should[:connmark]
# `time_start` and `time_stop` must be applied in full HH:MM:SS format
time = [:time_start, :time_stop]
diff -u -N -r ../lib/puppet/provider/firewallchain/firewallchain.rb lib/puppet/provider/firewallchain/firewallchain.rb
--- ../lib/puppet/provider/firewallchain/firewallchain.rb 2025-07-22 10:14:58.262456144 +0200
+++ lib/puppet/provider/firewallchain/firewallchain.rb 2025-07-22 10:15:08.710721654 +0200
@@ -9,18 +9,22 @@
# Command to list all chains and rules
$list_command = {
'IPv4' => 'iptables-save',
- 'IPv6' => 'ip6tables-save'
+ 'iptables' => 'iptables-save',
+ 'IPv6' => 'ip6tables-save',
+ 'ip6tables' => 'ip6tables-save'
}
# Regex used to divide output of$list_command between tables
$table_regex = %r{(\*(?:nat|mangle|filter|raw|rawpost|broute|security)[^*]+)}
- # Regex used to retrieve table name
- $table_name_regex = %r{^\*(nat|mangle|filter|raw|rawpost|broute|security)}
+ # Array of all the supported iptables
+ $supported_tables = ['nat', 'mangle', 'filter', 'raw', 'rawpost', 'broute', 'security']
# Regex used to retrieve Chains
$chain_regex = %r{\n:(INPUT|FORWARD|OUTPUT|(?:\S+))(?:\s(ACCEPT|DROP|QEUE|RETURN|PREROUTING|POSTROUTING))?}
# Base commands for the protocols, including table affixes
$base_command = {
'IPv4' => 'iptables -t',
- 'IPv6' => 'ip6tables -t'
+ 'iptables' => 'iptables -t',
+ 'IPv6' => 'ip6tables -t',
+ 'ip6tables' => 'ip6tables -t',
}
# Command to create a chain
$chain_create_command = '-N'
@@ -30,6 +34,10 @@
$chain_delete_command = '-X'
# Command to set chain policy, works on inbuilt chains only
$chain_policy_command = '-P'
+ # Command to list specific table so it will generate necessary output for iptables-save
+ # The retrieval of in-built chains may get confused by `iptables-save` tendency to not return table information
+ # for tables that have not yet been interacted with.
+ $table_list_command = '-L'
# Check if the given chain name references a built in one
$built_in_regex = %r{^(?:INPUT|OUTPUT|FORWARD|PREROUTING|POSTROUTING)$}
@@ -41,11 +49,10 @@
chains = []
# Scan String to retrieve all Chains and Policies
['IPv4', 'IPv6'].each do |protocol|
- # Retrieve String containing all IPv4 information
- iptables_list = Puppet::Provider.execute($list_command[protocol])
- iptables_list.scan($table_regex).each do |table|
- table_name = table[0].scan($table_name_regex)[0][0]
- table[0].scan($chain_regex).each do |chain|
+ # Go through each supported table and retrieve its chains if it exists.
+ $supported_tables.each do |table_name|
+ cmd_output = Puppet::Provider.execute([$list_command[protocol], '-t', table_name].join(' '), failonfail: false)
+ cmd_output.scan($chain_regex).each do |chain|
# Create the base hash
chain_hash = {
name: "#{chain[0]}:#{table_name}:#{protocol}",
@@ -94,7 +101,12 @@
def create(context, name, should)
context.notice("Creating Chain '#{name}' with #{should.inspect}")
- Puppet::Provider.execute([$base_command[should[:protocol]], should[:table], $chain_create_command, should[:chain]].join(' '))
+ # If a built-in chain is not present we assume that corresponding table has not been interacted with
+ if $built_in_regex.match(should[:chain])
+ Puppet::Provider.execute([$base_command[should[:protocol]], should[:table], $table_list_command].join(' '))
+ else
+ Puppet::Provider.execute([$base_command[should[:protocol]], should[:table], $chain_create_command, should[:chain]].join(' '))
+ end
PuppetX::Firewall::Utility.persist_iptables(context, name, should[:protocol])
end
@@ -150,10 +162,7 @@
should[:name] = should[:title] if should[:name].nil?
should[:chain], should[:table], should[:protocol] = should[:name].split(':')
- # If an in-built chain, always treat it as being present and ensure it is assigned a policy
- # The retrieval of in-built chains may get confused by `iptables-save` tendency to not return table information
- # for tables that have not yet been interacted with.
- is[:ensure] = 'present' if $built_in_regex.match(is[:chain])
+ # If an in-built chain, ensure it is assigned a policy
is[:policy] = 'accept' if $built_in_regex.match(is[:chain]) && is[:policy].nil?
# For the same reason assign it the default policy as an intended state if it does not have one
should[:policy] = 'accept' if $built_in_regex.match(should[:chain]) && should[:policy].nil?
@@ -172,7 +181,6 @@
raise ArgumentError, 'PREROUTING, POSTROUTING, INPUT, FORWARD and OUTPUT are the only inbuilt chains that can be used in table \'mangle\'' if %r{^(BROUTING)$}.match?(should[:chain])
when 'nat'
raise ArgumentError, 'PREROUTING, POSTROUTING, INPUT, and OUTPUT are the only inbuilt chains that can be used in table \'nat\'' if %r{^(BROUTING|FORWARD)$}.match?(should[:chain])
- raise ArgumentError, 'table nat isn\'t valid in IPv6. You must specify \':IPv4\' as the name suffix' if %r{^(IP(v6)?)?$}.match?(should[:protocol])
when 'raw'
raise ArgumentError, 'PREROUTING and OUTPUT are the only inbuilt chains in the table \'raw\'' if %r{^(POSTROUTING|BROUTING|INPUT|FORWARD)$}.match?(should[:chain])
when 'broute'
diff -u -N -r ../lib/puppet/type/firewall.rb lib/puppet/type/firewall.rb
--- ../lib/puppet/type/firewall.rb 2025-07-22 10:14:58.262456144 +0200
+++ lib/puppet/type/firewall.rb 2025-07-22 10:15:08.710721654 +0200
@@ -948,7 +948,7 @@
DESC
},
ipset: {
- type: 'Optional[Variant[Pattern[/^(?:!\s)?\w+\s(?:src|dst)(?:,src|,dst)?$/], Array[Pattern[/^(?:!\s)?\w+\s(?:src|dst)(?:,src|,dst)?$/]]]]',
+ type: 'Optional[Variant[Pattern[/^(?:!\s)?[\w\-:_]+\s(?:src|dst)(?:,src|,dst)?$/], Array[Pattern[/^(?:!\s)?[\w\-:_]+\s(?:src|dst)(?:,src|,dst)?$/]]]]',
desc: <<-DESC
Matches against the specified ipset list.
Requires ipset kernel module. Will accept a single element or an array.
@@ -1002,7 +1002,7 @@
* REJECT - the packet is rejected with a suitable ICMP response
* DROP - the packet is dropped
- But can also be on of the following:
+ But can also be one of the following:
* QUEUE
* RETURN
@@ -1189,7 +1189,7 @@
DESC
},
toports: {
- type: 'Optional[Pattern[/^\d+(?:-\d+)?$/]]',
+ type: 'Optional[Variant[Integer[0, 65535], Pattern[/^\d+(?:-\d+)?$/]]]',
desc: <<-DESC
For REDIRECT/MASQUERADE this is the port that will replace the destination/source port.
Can specify a single new port or an inclusive range of ports.
@@ -1261,13 +1261,15 @@
reject: {
type: "Optional[Enum['icmp-net-unreachable', 'icmp-host-unreachable', 'icmp-port-unreachable', 'icmp-proto-unreachable',
'icmp-net-prohibited', 'icmp-host-prohibited', 'icmp-admin-prohibited', 'icmp6-no-route', 'no-route',
- 'icmp6-adm-prohibited', 'adm-prohibited', 'icmp6-addr-unreachable', 'addr-unreach', 'icmp6-port-unreachable']]",
+ 'icmp6-adm-prohibited', 'adm-prohibited', 'icmp6-addr-unreachable', 'addr-unreach', 'icmp6-port-unreachable',
+ 'tcp-reset']]",
desc: <<-DESC
When combined with jump => "REJECT" you can specify a different icmp response to be sent back to the packet sender.
Valid values differ depending on if the protocol is `IPv4` or `IPv6`.
IPv4 allows: icmp-net-unreachable, icmp-host-unreachable, icmp-port-unreachable, icmp-proto-unreachable, icmp-net-prohibited,
- icmp-host-prohibited, or icmp-admin-prohibited.
- IPv6 allows: icmp6-no-route, no-route, icmp6-adm-prohibited, adm-prohibited, icmp6-addr-unreachable, addr-unreach, or icmp6-port-unreachable.
+ icmp-host-prohibited, icmp-admin-prohibited, or tcp-reset.
+ IPv6 allows: icmp6-no-route, no-route, icmp6-adm-prohibited, adm-prohibited, icmp6-addr-unreachable, addr-unreach,
+ icmp6-port-unreachable, or tcp-reset.
DESC
},
set_mark: {
@@ -1278,9 +1280,9 @@
DESC
},
match_mark: {
- type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]',
+ type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]',
desc: <<-DESC
- Match the Netfilter mark value associated with the packet, accepts a mark.
+ Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
This value will be converted to hex if it is not already.
This value can be negated by adding a space seperated `!` to the beginning.
DESC
@@ -1313,9 +1315,9 @@
DESC
},
connmark: {
- type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+$/]]',
+ type: 'Optional[Pattern[/^(?:!\s)?[a-fA-F0-9x]+(?:\/[a-fA-F0-9x]+)?$/]]',
desc: <<-DESC
- Match the Netfilter mark value associated with the packet, accepts a mark.
+ Match the Netfilter mark value associated with the packet. Accepts either of mark/mask or mark.
This value will be converted to hex if it is not already.
This value can be negated by adding a space seperated `!` to the beginning.
DESC
diff -u -N -r ../lib/puppet_x/puppetlabs/firewall/utility.rb lib/puppet_x/puppetlabs/firewall/utility.rb
--- ../lib/puppet_x/puppetlabs/firewall/utility.rb 2025-07-22 10:14:58.262456144 +0200
+++ lib/puppet_x/puppetlabs/firewall/utility.rb 2025-07-22 10:15:08.710721654 +0200
@@ -3,7 +3,7 @@
require 'puppet_x'
require 'socket'
require 'resolv'
-require 'puppet_x/puppetlabs/firewall/ipcidr'
+require_relative 'ipcidr'
module PuppetX::Firewall # rubocop:disable Style/ClassAndModuleChildren
# A utility class meant to contain re-usable code
@@ -216,25 +216,16 @@
# Accepts a valid mark or mark/mask and returns them in the valid
# hexidecimal format.
- # USed for set_mark
+ # Used for set_mark, match_mark, connmark
def self.mark_mask_to_hex(value)
- match = value.to_s.match(%r{([a-fA-F0-9x]+)/?([a-fA-F0-9x]+)?})
- mark = PuppetX::Firewall::Utility.to_hex32(match[1])
- return "#{mark}/0xffffffff" if match[2].nil?
-
- mask = PuppetX::Firewall::Utility.to_hex32(match[2])
- "#{mark}/#{mask}"
- end
-
- # Accepts a valid mark and returns them in the valid hexidecimal format.
- # Accounts for negation.
- # Used for match_mark / connmark
- def self.mark_to_hex(value)
- match = value.to_s.match(%r{^(!\s)?([a-fA-F0-9x]+)})
- mask = PuppetX::Firewall::Utility.to_hex32(match[2])
- return mask if match[1].nil?
+ match = value.to_s.match(%r{^(!\s)?([a-fA-F0-9x]+)\/?([a-fA-F0-9x]+)?})
+ negation = '! '
+ negation = '' if match[1].nil?
+ mark = PuppetX::Firewall::Utility.to_hex32(match[2])
+ return "#{negation}#{mark}/0xffffffff" if match[3].nil?
- "! #{mask}"
+ mask = PuppetX::Firewall::Utility.to_hex32(match[3])
+ "#{negation}#{mark}/#{mask}"
end
# Converts a given number to its protocol keyword
