]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
MODULES-1612 - sync src_range and dst_range
authorMorgan Haskel <morgan@puppetlabs.com>
Mon, 29 Dec 2014 21:41:06 +0000 (13:41 -0800)
committerMorgan Haskel <morgan@puppetlabs.com>
Mon, 29 Dec 2014 21:47:24 +0000 (13:47 -0800)
The the firewall type and the ip6tables provider did not support
src_range and dst_range for ip6tables. Added this functionality.

README.markdown
lib/puppet/provider/firewall/ip6tables.rb
lib/puppet/type/firewall.rb
spec/acceptance/firewall_spec.rb

index abd85b40d3c69809b5e580babe52f10b3fde6953..362a74e4385bef13f1f51dd886ac42faaaa74df5 100644 (file)
@@ -439,7 +439,7 @@ If Puppet is managing the iptables or iptables-persistent packages, and the prov
 
 * `dst_range`: The destination IP range. For example: `dst_range => '192.168.1.1-192.168.1.10'`.
 
-  The destination IP range is must in 'IP1-IP2' format. Values must match '0.0.0.0-0.0.0.0' through '255.255.255.255-255.255.255.255'. Requires the `iprange` feature.
+  The destination IP range is must in 'IP1-IP2' format. Values in the range must be valid IPv4 or IPv6 addresses. Requires the `iprange` feature.
 
 * `dst_type`: The destination address type. For example: `dst_type => 'LOCAL'`.
 
@@ -588,7 +588,7 @@ firewall { '101 blacklist strange traffic':
 
 * `sport`: The source port to match for this filter (if the protocol supports ports). Will accept a single element or an array. For some firewall providers you can pass a range of ports in the format:'start number-end number'. For example, '1-1024' would cover ports 1 to 1024.
 
-* `src_range`: The source IP range. For example: `src_range => '192.168.1.1-192.168.1.10'`. The source IP range is must in 'IP1-IP2' format. Values must match '0.0.0.0-0.0.0.0' through '255.255.255.255-255.255.255.255'. Requires the `iprange` feature.
+* `src_range`: The source IP range. For example: `src_range => '192.168.1.1-192.168.1.10'`. The source IP range must be in 'IP1-IP2' format. Values in the range must be valid IPv4 or IPv6 addresses. Requires the `iprange` feature.
 
 * `src_type`: Specify the source address type. For example: `src_type => 'LOCAL'`.
 
index d58b86d9442443a23c56dadbf5680159aa4d9e4b..afc5171d17e4d921b09158b66f6be7ef0e65c085 100644 (file)
@@ -21,6 +21,7 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
   has_feature :ishasmorefrags
   has_feature :islastfrag
   has_feature :isfirstfrag
+  has_feature :iprange
 
   optional_commands({
     :ip6tables      => 'ip6tables',
@@ -55,6 +56,7 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
     :ctstate          => "-m conntrack --ctstate",
     :destination      => "-d",
     :dport            => ["-m multiport --dports", "--dport"],
+    :dst_range        => '-m iprange --dst-range',
     :gid              => "-m owner --gid-owner",
     :hop_limit        => "-m hl --hl-eq",
     :icmp             => "-m icmp6 --icmpv6-type",
@@ -82,6 +84,7 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
     :rttl             => "--rttl",
     :source           => "-s",
     :sport            => ["-m multiport --sports", "--sport"],
+    :src_range        => '-m iprange --src-range',
     :stat_every       => '--every',
     :stat_mode        => "-m statistic --mode",
     :stat_packet      => '--packet',
@@ -134,7 +137,8 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
   # I put it when calling the command. So compability with manual changes
   # not provided with current parser [georg.koester])
   @resource_list = [:table, :source, :destination, :iniface, :outiface,
-    :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :tcp_flags, :gid, :uid, :sport, :dport,
+    :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :src_range, :dst_range,
+    :tcp_flags, :gid, :uid, :sport, :dport,
     :port, :pkttype, :name, :state, :ctstate, :icmp, :hop_limit, :limit, :burst,
     :recent, :rseconds, :reap, :rhitcount, :rttl, :rname, :rsource, :rdest,
     :jump, :todest, :tosource, :toports, :log_level, :log_prefix, :reject,
index 85c6464e36d10c22985bcf7a05cacfaebd445e37..79a94a9e09da19932ab8794c554281736639973b 100644 (file)
@@ -137,10 +137,25 @@ Puppet::Type.newtype(:firewall) do
 
           src_range => '192.168.1.1-192.168.1.10'
 
-      The source IP range is must in 'IP1-IP2' format.
+      The source IP range must be in 'IP1-IP2' format.
     EOS
 
-    newvalues(/^((25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)-((25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)/)
+    validate do |value|
+      if matches = /^([^\-\/]+)-([^\-\/]+)$/.match(value)
+        start_addr = matches[1]
+        end_addr = matches[2]
+
+        [start_addr, end_addr].each do |addr|
+          begin
+            @resource.host_to_ip(addr)
+          rescue Exception
+            self.fail("Invalid IP address \"#{addr}\" in range \"#{value}\"")
+          end
+        end
+      else
+        raise(ArgumentError, "The source IP range must be in 'IP1-IP2' format.")
+      end
+    end
   end
 
   newproperty(:destination) do
@@ -172,10 +187,25 @@ Puppet::Type.newtype(:firewall) do
 
           dst_range => '192.168.1.1-192.168.1.10'
 
-      The destination IP range is must in 'IP1-IP2' format.
+      The destination IP range must be in 'IP1-IP2' format.
     EOS
 
-    newvalues(/^((25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)-((25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)\.){3}(25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)/)
+    validate do |value|
+      if matches = /^([^\-\/]+)-([^\-\/]+)$/.match(value)
+        start_addr = matches[1]
+        end_addr = matches[2]
+
+        [start_addr, end_addr].each do |addr|
+          begin
+            @resource.host_to_ip(addr)
+          rescue Exception
+            self.fail("Invalid IP address \"#{addr}\" in range \"#{value}\"")
+          end
+        end
+      else
+        raise(ArgumentError, "The destination IP range must be in 'IP1-IP2' format.")
+      end
+    end
   end
 
   newproperty(:sport, :array_matching => :all) do
index 06e47c6cdd9a606ef627fcfd15c86bd9dcecf50c..5842b91ae89a27313c00cf3dbf56995cd9e5c658 100644 (file)
@@ -219,7 +219,7 @@ describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfami
         EOS
 
         apply_manifest(pp, :expect_failures => true) do |r|
-          expect(r.stderr).to match(/Invalid value "392.168.1.1-192.168.1.10"/)
+          expect(r.stderr).to match(/Invalid IP address "392.168.1.1" in range "392.168.1.1-192.168.1.10"/)
         end
       end
 
@@ -348,7 +348,7 @@ describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfami
         EOS
 
         apply_manifest(pp, :expect_failures => true) do |r|
-          expect(r.stderr).to match(/Invalid value "392.168.1.1-192.168.1.10"/)
+          expect(r.stderr).to match(/Invalid IP address "392.168.1.1" in range "392.168.1.1-192.168.1.10"/)
         end
       end
 
@@ -1116,6 +1116,113 @@ describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfami
         end
       end
     end
+
+    describe 'src_range' do
+      context '2001:db8::1-2001:db8::ff' do
+        it 'applies' do
+          pp = <<-EOS
+          class { '::firewall': }
+          firewall { '601 - test':
+            proto  => tcp,
+            port   => '601',
+            action => accept,
+            src_range => '2001:db8::1-2001:db8::ff',
+            provider => 'ip6tables',
+          }
+          EOS
+
+          apply_manifest(pp, :catch_failures => true)
+          unless fact('selinux') == 'true'
+            apply_manifest(pp, :catch_changes => true)
+          end
+        end
+
+        it 'should contain the rule' do
+          shell('ip6tables-save') do |r|
+            expect(r.stdout).to match(/-A INPUT -p tcp -m iprange --src-range 2001:db8::1-2001:db8::ff -m multiport --ports 601 -m comment --comment "601 - test" -j ACCEPT/)
+          end
+        end
+      end
+
+      # Invalid IP
+      context '2001::db8::1-2001:db8::ff' do
+        it 'applies' do
+          pp = <<-EOS
+          class { '::firewall': }
+          firewall { '601 - test':
+            proto  => tcp,
+            port   => '601',
+            action => accept,
+            src_range => '2001::db8::1-2001:db8::ff',
+          }
+          EOS
+
+          apply_manifest(pp, :expect_failures => true) do |r|
+            expect(r.stderr).to match(/Invalid IP address "2001::db8::1" in range "2001::db8::1-2001:db8::ff"/)
+          end
+        end
+
+        it 'should not contain the rule' do
+          shell('iptables-save') do |r|
+            expect(r.stdout).to_not match(/-A INPUT -p tcp -m iprange --src-range 2001::db8::1-2001:db8::ff -m multiport --ports 601 -m comment --comment "601 - test" -j ACCEPT/)
+          end
+        end
+      end
+    end
+
+    describe 'dst_range' do
+      context '2001:db8::1-2001:db8::ff' do
+        it 'applies' do
+          pp = <<-EOS
+          class { '::firewall': }
+          firewall { '602 - test':
+            proto  => tcp,
+            port   => '602',
+            action => accept,
+            dst_range => '2001:db8::1-2001:db8::ff',
+            provider => 'ip6tables',
+          }
+          EOS
+
+          apply_manifest(pp, :catch_failures => true)
+          unless fact('selinux') == 'true'
+            apply_manifest(pp, :catch_changes => true)
+          end
+        end
+
+        it 'should contain the rule' do
+          shell('ip6tables-save') do |r|
+            expect(r.stdout).to match(/-A INPUT -p tcp -m iprange --dst-range 2001:db8::1-2001:db8::ff -m multiport --ports 602 -m comment --comment "602 - test" -j ACCEPT/)
+          end
+        end
+      end
+
+      # Invalid IP
+      context '2001::db8::1-2001:db8::ff' do
+        it 'applies' do
+          pp = <<-EOS
+          class { '::firewall': }
+          firewall { '602 - test':
+            proto  => tcp,
+            port   => '602',
+            action => accept,
+            dst_range => '2001::db8::1-2001:db8::ff',
+          }
+          EOS
+
+          apply_manifest(pp, :expect_failures => true) do |r|
+            expect(r.stderr).to match(/Invalid IP address "2001::db8::1" in range "2001::db8::1-2001:db8::ff"/)
+          end
+        end
+
+        it 'should not contain the rule' do
+          shell('iptables-save') do |r|
+            expect(r.stdout).to_not match(/-A INPUT -p tcp -m iprange --dst-range 2001::db8::1-2001:db8::ff -m multiport --ports 602 -m comment --comment "602 - test" -j ACCEPT/)
+          end
+        end
+      end
+    end
+
   end
 
   describe 'limit' do