Merge pull request #1126 from greatflyingsteve/support-tcp-option

Add support for parsing and using --tcp-option

Author: LukasAud

.rubocop.yml

@@ -26,6 +26,10 @@ AllCops:
 Layout/LineLength:
   Description: People have wide screens, use them.
   Max: 200
+Metrics/BlockLength:
+  AllowedMethods:
+    - provide
+    - newtype
 RSpec/BeforeAfterAll:
   Description: Beware of using after(:all) as it may cause state to leak between tests.
     A necessary evil in acceptance testing.
@@ -83,4 +87,4 @@ Style/Documentation:
   - lib/puppet/parser/functions/**/*
   - spec/**/*
 Style/WordArray:
-  EnforcedStyle: brackets
+  EnforcedStyle: brackets

.rubocop_todo.yml

@@ -114,11 +114,11 @@ Lint/RedundantSafeNavigation:
 Metrics/AbcSize:
   Max: 235
 
-# Offense count: 23
+# Offense count: 17
 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode.
 # AllowedMethods: refine
 Metrics/BlockLength:
-  Max: 1961
+  Max: 64
 
 # Offense count: 2
 # Configuration parameters: CountBlocks.

lib/puppet/provider/firewall/ip6tables.rb

@@ -29,6 +29,7 @@
   has_feature :nflog_prefix
   has_feature :nflog_range
   has_feature :nflog_threshold
+  has_feature :tcp_option
   has_feature :tcp_flags
   has_feature :pkttype
   has_feature :ishasmorefrags
@@ -183,7 +184,8 @@ def self.iptables_save(*args)
     string_from: '--from',
     string_to: '--to',
     table: '-t',
-    tcp_flags: '-m tcp --tcp-flags',
+    tcp_option: '--tcp-option',
+    tcp_flags: '--tcp-flags',
     todest: '--to-destination',
     toports: '--to-ports',
     tosource: '--to-source',
@@ -312,7 +314,7 @@ def self.iptables_save(*args)
   @resource_list = [:table, :source, :destination, :iniface, :outiface, :physdev_in,
                     :physdev_out, :physdev_is_bridged, :physdev_is_in, :physdev_is_out,
                     :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :src_range, :dst_range,
-                    :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port, :src_type,
+                    :tcp_option, :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port, :src_type,
                     :dst_type, :socket, :pkttype, :ipsec_dir, :ipsec_policy, :state,
                     :ctstate, :ctproto, :ctorigsrc, :ctorigdst, :ctreplsrc, :ctrepldst,
                     :ctorigsrcport, :ctorigdstport, :ctreplsrcport, :ctrepldstport, :ctstatus, :ctexpire, :ctdir,

lib/puppet/provider/firewall/iptables.rb

@@ -34,6 +34,7 @@
   has_feature :nflog_prefix
   has_feature :nflog_range
   has_feature :nflog_threshold
+  has_feature :tcp_option
   has_feature :tcp_flags
   has_feature :pkttype
   has_feature :isfragment
@@ -173,7 +174,8 @@
     string_from: '--from',
     string_to: '--to',
     table: '-t',
-    tcp_flags: ['-m tcp --tcp-flags', '--tcp-flags'],
+    tcp_option: '--tcp-option',
+    tcp_flags: '--tcp-flags',
     todest: '--to-destination',
     toports: '--to-ports',
     tosource: '--to-source',
@@ -296,7 +298,13 @@ def self.munge_resource_map_from_existing_values(resource_map_original, compare)
 
   def munge_resource_map_from_resource(resource_map_original, compare)
     resource_map_new = resource_map_original.clone
-    module_to_argument_mapping = self.class.instance_variable_get('@module_to_argument_mapping')
+    # We ignore the 'tcp' match module on rule parse, but it needs to be included to generate
+    # arguments, since both '--tcp-option' and '--tcp-flags' should be prefixed with the match
+    # module spec.  '--dport' and '--sport' are ignored because we only produce multiport-style
+    # portspecs, which do not require '-m tcp', and for which iptables-save does not include
+    # '-m tcp' in its output.
+    tcp_module_arguments = { tcp: [:tcp_option, :tcp_flags] }
+    module_to_argument_mapping = self.class.instance_variable_get('@module_to_argument_mapping').merge(tcp_module_arguments)
 
     module_to_argument_mapping.each do |ipt_module, arg_array|
       arg_array.each do |argument|
@@ -348,7 +356,7 @@ def munge_resource_map_from_resource(resource_map_original, compare)
     :table, :source, :destination, :iniface, :outiface,
     :physdev_in, :physdev_out, :physdev_is_bridged, :physdev_is_in, :physdev_is_out,
     :proto, :isfragment, :stat_mode, :stat_every, :stat_packet, :stat_probability,
-    :src_range, :dst_range, :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port,
+    :src_range, :dst_range, :tcp_option, :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port,
     :src_type, :dst_type, :socket, :pkttype, :ipsec_dir, :ipsec_policy,
     :state, :ctstate, :ctproto, :ctorigsrc, :ctorigdst, :ctreplsrc, :ctrepldst,
     :ctorigsrcport, :ctorigdstport, :ctreplsrcport, :ctrepldstport, :ctstatus, :ctexpire, :ctdir,
@@ -507,8 +515,12 @@ def self.rule_to_hash(line, table, counter)
     values = values.gsub(%r{(?<=\s)(-\S+) (!)\s?(\S*)}, '\1 "\2 \3"')
     # fix negated physdev rules
     values = values.gsub(%r{-m physdev ! (--physdev-is-\S+)}, '-m physdev \1 "!"')
-    # The match extension for tcp & udp are optional and throws off the @resource_map.
-    values = values.gsub(%r{(?!-m tcp --tcp-flags)-m (tcp|udp) }, '')
+    # The match extensions for tcp & udp are implied by the protocol, cannot be
+    # given unless the protocol matches the extension name, are never required,
+    # and if multiport matches are used, may not even be in the iptables-save
+    # output in predictable locations.  They need to be removed to preserve
+    # parse sanity.
+    values = values.gsub(%r{-m (tcp|udp) }, '')
     # There is a bug in EL5 which puts 2 spaces before physdev, so we fix it
     values = values.gsub(%r{\s{2}--physdev}, ' --physdev')
     # '--pol ipsec' takes many optional arguments; we cheat again by adding " around them

lib/puppet/type/firewall.rb

@@ -134,6 +134,8 @@
 
       * string_matching: The ability to match a given string by using some pattern matching strategy.
 
+      * tcp_option: The ability to match on particular TCP options.
+
       * tcp_flags: The ability to match on particular TCP flag settings.
 
       * netmap: The ability to map entire subnets via source or destination nat rules.
@@ -171,6 +173,7 @@
   feature :log_ip_options, 'Add IP/IPv6 packet header to log messages'
   feature :mark, 'Match or Set the netfilter mark value associated with the packet'
   feature :mss, 'Match a given TCP MSS value or range.'
+  feature :tcp_option, 'The ability to match on particular TCP options'
   feature :tcp_flags, 'The ability to match on particular TCP flag settings'
   feature :pkttype, 'Match a packet type'
   feature :rpfilter, 'Perform reverse-path filtering'
@@ -569,6 +572,24 @@ def should_to_s(value)
     PUPPETCODE
   end
 
+  # tcp-specific
+  newproperty(:tcp_option, required_features: :tcp_option) do
+    desc <<-PUPPETCODE
+      Match when the TCP option is present or absent.
+      Given as a single TCP option, optionally prefixed with '! ' to match
+      on absence instead.  Only one TCP option can be matched in a given rule.
+      TCP option numbers are an eight-bit field, so valid option numbers range
+      from 0-255.
+    PUPPETCODE
+
+    validate do |value|
+      unless value.to_i.bit_length < 8 && value.to_i >= 0
+        raise ArgumentError, "TCP Options fall in the range 0-255, #{value} is not a valid TCP Option number"
+      end
+    end
+    munge { |value| value.to_s }
+  end
+
   # tcp-specific
   newproperty(:tcp_flags, required_features: :tcp_flags) do
     desc <<-PUPPETCODE
@@ -2341,7 +2362,7 @@ def should_to_s(value)
     ['/etc/sysconfig/iptables', '/etc/sysconfig/ip6tables']
   end
 
-  validate do
+  validate do # rubocop:disable Metrics/BlockLength
     debug('[validate]')
 
     # TODO: this is put here to skip validation if ensure is not set. This

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "puppetlabs-firewall",
-  "version": "5.0.0",
+  "version": "5.1.0",
   "author": "puppetlabs",
   "summary": "Manages Firewalls such as iptables",
   "license": "Apache-2.0",

spec/fixtures/ip6tables/conversion_hash.rb

@@ -49,7 +49,94 @@
     params: {
       random_fully: 'true',
     }
-  }
+  },
+  'tcp_flags_1' => {
+    line: '-A INPUT -p tcp -m tcp --tcp-flags SYN,RST,ACK,FIN SYN -m comment --comment "000 initiation"',
+    compare_all: true,
+    table: 'filter',
+    chain: 'INPUT',
+    proto: 'tcp',
+    params: {
+      name: '000 initiation',
+      tcp_flags: 'SYN,RST,ACK,FIN SYN',
+      proto: 'tcp',
+      chain: 'INPUT',
+      line: '-A INPUT -p tcp -m tcp --tcp-flags SYN,RST,ACK,FIN SYN -m comment --comment "000 initiation"',
+      provider: 'ip6tables',
+      table: 'filter',
+      ensure: :present,
+    },
+  },
+  'tcp_option_1' => {
+    line: '-A INPUT -p tcp -m tcp --tcp-option 8 -m comment --comment "001 tcp_option works alone"',
+    compare_all: true,
+    table: 'filter',
+    chain: 'INPUT',
+    proto: 'tcp',
+    params: {
+      chain: 'INPUT',
+      ensure: :present,
+      line: '-A INPUT -p tcp -m tcp --tcp-option 8 -m comment --comment "001 tcp_option works alone"',
+      name: '001 tcp_option works alone',
+      proto: 'tcp',
+      provider: 'ip6tables',
+      table: 'filter',
+      tcp_option: '8',
+    },
+  },
+  'tcp_option_2' => {
+    line: '-A INPUT -p tcp -m tcp ! --tcp-option 8 -m comment --comment "002 tcp_option works alone, negated"',
+    compare_all: true,
+    table: 'filter',
+    chain: 'INPUT',
+    proto: 'tcp',
+    params: {
+      chain: 'INPUT',
+      ensure: :present,
+      line: '-A INPUT -p tcp -m tcp ! --tcp-option 8 -m comment --comment "002 tcp_option works alone, negated"',
+      name: '002 tcp_option works alone, negated',
+      proto: 'tcp',
+      provider: 'ip6tables',
+      table: 'filter',
+      tcp_option: '! 8',
+    },
+  },
+  'tcp_option_with_tcp_flags_1' => {
+    line: '-A INPUT -p tcp -m tcp --tcp-option 8 --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "000 initiation"',
+    table: 'filter',
+    compare_all: true,
+    chain: 'INPUT',
+    proto: 'tcp',
+    params: {
+      chain: 'INPUT',
+      ensure: :present,
+      line: '-A INPUT -p tcp -m tcp --tcp-option 8 --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "000 initiation"',
+      name: '000 initiation',
+      proto: 'tcp',
+      provider: 'ip6tables',
+      table: 'filter',
+      tcp_flags: 'FIN,SYN,RST,ACK SYN',
+      tcp_option: '8',
+    },
+  },
+  'tcp_option_with_tcp_flags_2' => {
+    line: '-A INPUT -p tcp -m tcp ! --tcp-option 8 --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "000 initiation"',
+    table: 'filter',
+    compare_all: true,
+    chain: 'INPUT',
+    proto: 'tcp',
+    params: {
+      chain: 'INPUT',
+      ensure: :present,
+      line: '-A INPUT -p tcp -m tcp ! --tcp-option 8 --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "000 initiation"',
+      name: '000 initiation',
+      proto: 'tcp',
+      provider: 'ip6tables',
+      table: 'filter',
+      tcp_flags: 'FIN,SYN,RST,ACK SYN',
+      tcp_option: '! 8',
+    },
+  },
 }.freeze
 
 # This hash is for testing converting a hash to an argument line.
@@ -141,4 +228,55 @@
     },
     args: ['-t', :filter, '-p', :tcp, '-j', 'NFLOG', '--nflog-group', 1, '--nflog-prefix', 'myprefix', '-m', 'comment', '--comment', '100 nflog'],
   },
+  'tcp_flags_1' => {
+    params: {
+      name: '000 initiation',
+      tcp_flags: 'SYN,RST,ACK,FIN SYN',
+      table: 'filter',
+    },
+
+    args: ['-t', :filter, '-p', :tcp, '-m', 'tcp', '--tcp-flags', 'SYN,RST,ACK,FIN', 'SYN', '-m', 'comment', '--comment', '000 initiation'],
+  },
+  'tcp_option_1' => {
+    params: {
+      name: '000 initiation',
+      table: 'filter',
+      chain: 'INPUT',
+      proto: 'tcp',
+      tcp_option: '8',
+    },
+    args: ['-t', :filter, '-p', :tcp, '-m', 'tcp', '--tcp-option', '8', '-m', 'comment', '--comment', '000 initiation'],
+  },
+  'tcp_option_2' => {
+    params: {
+      name: '000 initiation',
+      table: 'filter',
+      chain: 'INPUT',
+      proto: 'tcp',
+      tcp_option: '! 8',
+    },
+    args: ['-t', :filter, '-p', :tcp, '-m', 'tcp', '!', '--tcp-option', '8', '-m', 'comment', '--comment', '000 initiation'],
+  },
+  'tcp_option_with_tcp_flags_1' => {
+    params: {
+      name: '000 initiation',
+      table: 'filter',
+      chain: 'INPUT',
+      proto: 'tcp',
+      tcp_flags: 'FIN,SYN,RST,ACK SYN',
+      tcp_option: '8',
+    },
+    args: ['-t', :filter, '-p', :tcp, '-m', 'tcp', '--tcp-option', '8', '--tcp-flags', 'FIN,SYN,RST,ACK', 'SYN', '-m', 'comment', '--comment', '000 initiation'],
+  },
+  'tcp_option_with_tcp_flags_2' => {
+    params: {
+      name: '000 initiation',
+      table: 'filter',
+      chain: 'INPUT',
+      proto: 'tcp',
+      tcp_flags: 'FIN,SYN,RST,ACK SYN',
+      tcp_option: '! 8',
+    },
+    args: ['-t', :filter, '-p', :tcp, '-m', 'tcp', '!', '--tcp-option', '8', '--tcp-flags', 'FIN,SYN,RST,ACK', 'SYN', '-m', 'comment', '--comment', '000 initiation'],
+  },
 }.freeze