From fbc76178cd3e45ab294067dac7504b678450a6eb Mon Sep 17 00:00:00 2001
From: Ken Barber <ken@bob.sh>
Date: Sat, 16 Jul 2011 21:19:49 +0200
Subject: [PATCH] Removed iptables stuff, introduced features and cleaned up
 docs & validation.

I've removed a lot of iptables specific stuff from the type, also
allowed any chain to be defined in :chain or :jump so we can support
user chains.

A lot of the documentation for the type has been cleaned up a little
and validation has been simplified where applicable.

This commit brings in the usage of features so we can start to introduce
more backend providers. The work is just a start for now and will
probably radically change once we have other working providers in
place.
---
 lib/puppet/provider/firewall/iptables.rb |  16 +-
 lib/puppet/type/firewall.rb              | 187 ++++++++++++-----------
 spec/type/iptables_type_spec.rb          |   2 +-
 3 files changed, 112 insertions(+), 93 deletions(-)

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 84238a8..9947b2d 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -5,6 +5,17 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
   
   @doc = "Iptables type provider"
 
+  has_feature :iptables
+  has_feature :rate_limiting
+  has_feature :snat
+  has_feature :dnat
+  has_feature :interface_match
+  has_feature :icmp_match
+  has_feature :state_match
+  has_feature :reject_type
+  has_feature :log_level
+  has_feature :log_prefix
+
   commands :iptables => '/sbin/iptables'
   commands :iptables_save => '/sbin/iptables-save'
 
@@ -34,8 +45,9 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
     :tosource => "--to-source",
   }
 
-  @@resource_list = [:table, :source, :destination, :iniface, :outiface, :proto, :sport, :dport, :tosource, :todest,
-                     :reject, :log_level, :log_prefix, :name, :state, :icmp, :limit, :burst, :toports, :jump]
+  @@resource_list = [:table, :source, :destination, :iniface, :outiface, 
+    :proto, :sport, :dport, :reject, :log_level, :log_prefix, :name, :state, 
+    :icmp, :limit, :burst, :toports, :jump, :todest, :tosource]
 
 
   def insert
diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 7dbbe9f..3438c2b 100644
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -4,16 +4,32 @@ require 'puppet/util/firewall'
 Puppet::Type.newtype(:firewall) do
   include Puppet::Util::Firewall
 
-  @doc = "Manipulate firewall rules"
+  @doc = "This type provides the capability to manage firewall rules within 
+          puppet."
+
+  feature :rate_limiting, "Rate limiting features."
+  feature :snat, "Source NATing"
+  feature :dnat, "Destination NATing"
+  feature :interface_match, "Interface matching"
+  feature :icmp_match, "Matching ICMP types"
+  feature :state_match, "Matching stateful firewall states"
+  feature :reject_type, "The ability to control reject messages"
+  feature :log_level, "The ability to control the log level"
+  feature :log_prefix, "The ability to add prefixes to log messages"
+
+  # provider specific features
+  feature :iptables, "The provider provides iptables features."
 
   ensurable do
-    desc "Create or remove this rule."
+    desc "Manage the state of this rule."
 
     newvalue(:present) do
+      desc "Ensure the rule is present"
       provider.insert
     end
 
     newvalue(:absent) do
+      desc "Ensure the rule is absent"
       provider.delete
     end
 
@@ -21,60 +37,16 @@ Puppet::Type.newtype(:firewall) do
   end
 
   newparam(:name) do
-    desc "The name of the rule."
+    desc "The canonical name of the rule."
     isnamevar
 
-    # Keep rule names simple
-    validate do |value|
-      if value !~ /[a-zA-Z0-9 \-_]+/
-        self.fail "Not a valid rule name. Make sure it contains ASCII " \
-          "alphanumeric, spaces, hyphens or underscores."
-      end
-
-      if value !~ /^\d+/
-        self.fail 'Not a valid rule name. Rules names must start with a digit'
-      end
-    end
-  end
-
-  newproperty(:chain) do
-    desc "The value for the iptables -A parameter.
-      Possible values are: 'INPUT', 'FORWARD', 'OUTPUT', 'PREROUTING', 'POSTROUTING'.
-      Default value is 'INPUT'"
-    newvalues(:INPUT, :FORWARD, :OUTPUT, :PREROUTING, :POSTROUTING)
-    defaultto "INPUT"
-  end
-
-  newproperty(:table) do
-    desc "The value for the iptables -t parameter.
-      Possible values are: 'nat', 'mangle', 'filter' and 'raw'.
-      Default value is 'filter'"
-    newvalues(:nat, :mangle, :filter, :raw)
-    defaultto "filter"
-  end
-
-  newproperty(:proto) do
-    desc "The value for the iptables --protocol parameter.
-      Possible values are: 'tcp', 'udp', 'icmp', 'esp', 'ah', 'vrrp',
-      'igmp', 'all'.
-      Default value is 'tcp'"
-    newvalues(:tcp, :udp, :icmp, :esp, :ah, :vrrp, :igmp, :all)
-    defaultto "tcp"
-  end
-
-  newproperty(:jump) do
-    desc "The value for the iptables --jump parameter.
-      Possible values are: 'ACCEPT', 'DROP', 'QUEUE', 'RETURN',
-      REJECT', 'DNAT', 'SNAT', 'LOG', 'MASQUERADE', 'REDIRECT'.
-      Default value is 'ACCEPT'"
-    newvalues(:ACCEPT, :DROP, :QUEUE, :RETURN, :REJECT, :DNAT, :SNAT, :LOG,
-              :MASQUERADE, :REDIRECT)
-    defaultto "ACCEPT"
+    # Keep rule names simple - they must start with a number
+    newvalues(/^\d+[a-zA-Z0-9\s\-_]+$/)
   end
 
+  # Generic matching properties
   newproperty(:source, :array_matching => :all) do
-    desc "The value for the iptables --source parameter.
-      Accepts a single string or array."
+    desc "The source IP address to match. Accepts a string or array."
 
     def should_to_s(value)
       value = [value] unless value.is_a?(Array)
@@ -83,8 +55,7 @@ Puppet::Type.newtype(:firewall) do
   end
 
   newproperty(:destination, :array_matching => :all) do
-    desc "The value for the iptables --destination parameter.
-      Accepts a single string or array."
+    desc "The destination IP address to match. Accepts a string or array."
 
     def should_to_s(value)
       value = [value] unless value.is_a?(Array)
@@ -93,8 +64,8 @@ Puppet::Type.newtype(:firewall) do
   end
 
   newproperty(:sport, :array_matching => :all) do
-    desc "The value for the iptables --source-port parameter.
-      If an array is specified, values will be passed to multiport module."
+    desc "The source port to match for this filter (if the protocol supports 
+          ports). Will accept a single element or an array."
 
     munge do |value|
       @resource.string_to_port(value)
@@ -107,8 +78,8 @@ Puppet::Type.newtype(:firewall) do
   end
 
   newproperty(:dport, :array_matching => :all) do
-    desc "The value for the iptables --destination-port parameter.
-      If an array is specified, values will be passed to multiport module."
+    desc "The destination port to match for this filter (if the protocol 
+          supports ports). Will accept a single element or an array."
     
     munge do |value|
       @resource.string_to_port(value)
@@ -120,40 +91,79 @@ Puppet::Type.newtype(:firewall) do
     end
   end
 
-  newproperty(:iniface) do
-    desc "The value for the iptables --in-interface parameter"
+  newproperty(:proto) do
+    desc "The specific protocol to match for this rule."
+    newvalues(:tcp, :udp, :icmp, :esp, :ah, :vrrp, :igmp, :all)
+    defaultto "tcp"
+  end
+
+  # Iptables specific
+  newproperty(:chain, :required_features => :iptables) do
+    desc "The value for the iptables -A parameter. Normal values are: 'INPUT', 
+          'FORWARD', 'OUTPUT', 'PREROUTING', 'POSTROUTING' but you can also
+          specify a user created chain."
+
+    defaultto "INPUT"
+    newvalue(/^[a-zA-Z0-9\-_]+$/)
+  end
+
+  newproperty(:table, :required_features => :iptables) do
+    desc "The value for the iptables -t parameter."
+    newvalues(:nat, :mangle, :filter, :raw, :rawpost)
+    defaultto "filter"
   end
 
-  newproperty(:outiface) do
-    desc "The value for the iptables --out-interface parameter"
+  newproperty(:jump, :required_features => :iptables) do
+    desc "The value for the iptables --jump parameter. Normal values are: 
+          'ACCEPT', 'DROP', 'QUEUE', 'RETURN', REJECT', 'DNAT', 'SNAT', 'LOG', 
+          'MASQUERADE', 'REDIRECT'. But any valid chain name is allowed."
+    newvalues(/^[a-zA-Z0-9\-_]+$/)
+    defaultto "ACCEPT"
+  end
+
+  # Interface specific matching properties
+  newproperty(:iniface, :required_features => :interface_match) do
+    desc "Match input interface."
+    newvalues(/^[a-zA-Z0-9\-_]+$/)
+  end
+
+  newproperty(:outiface, :required_features => :interface_match) do
+    desc "Match ouput interface."
+    newvalues(/^[a-zA-Z0-9\-_]+$/)
   end
 
-  newproperty(:tosource) do
-    desc "The value for the iptables --to-source parameter"
+  # NAT specific properties
+  newproperty(:tosource, :required_features => :snat) do
+    desc "For SNAT this is the IP address that will replace the source IP 
+          address."
   end
 
-  newproperty(:todest) do
-    desc "The value for the iptables --to-destination parameter"
+  newproperty(:todest, :required_features => :dnat) do
+    desc "For DNAT this is the IP address that will replace the destination IP 
+          address."
   end
 
-  newproperty(:toports) do
-    desc "The value for the iptables --to-ports parameter"
+  newproperty(:toports, :required_features => :dnat) do
+    desc "For DNAT this is the port that will replace the destination port."
   end
 
-  newproperty(:reject) do
-    desc "The value for the iptables --reject-with parameter"
+  # Reject ICMP type
+  newproperty(:reject, :required_features => :reject_type) do
+    desc "The ICMP response to reject a packet with."
   end
 
-  newproperty(:log_level) do
-    desc "The value for the iptables --log-level parameter"
+  # Logging properties
+  newproperty(:log_level, :required_features => :log_level) do
+    desc "The syslog level to log to."
   end
 
-  newproperty(:log_prefix) do
-    desc "The value for the iptables --log-level parameter"
+  newproperty(:log_prefix, :required_features => :log_prefix) do
+    desc "The syslog prefix."
   end
 
-  newproperty(:icmp) do
-    desc "The value for the iptables --icmp-type parameter"
+  # ICMP matching property
+  newproperty(:icmp, :required_features => :icmp_match) do
+    desc "When matching ICMP packets, this is the type of ICMP packet to match."
 
     munge do |value|
       if value.kind_of?(String)
@@ -169,10 +179,11 @@ Puppet::Type.newtype(:firewall) do
     end
   end
 
-  newproperty(:state, :array_matching => :all) do
-    desc "The value for the iptables -m state --state parameter.
-      Possible values are: 'INVALID', 'ESTABLISHED', 'NEW', 'RELATED'.
-      Accepts a single string or array."
+  newproperty(:state, :array_matching => :all, :required_features => :state_match) do
+    desc "Matches a packet based on its state in the firewall stateful inspection 
+          table."
+
+    newvalues(:INVALID,:ESTABLISHED,:NEW,:RELATED)
 
     def should_to_s(value)
       value = [value] unless value.is_a?(Array)
@@ -180,19 +191,15 @@ Puppet::Type.newtype(:firewall) do
     end
   end
 
-  newproperty(:limit) do
-    desc "The value for the iptables -m limit --limit parameter.
-      Example values are: '50/sec', '40/min', '30/hour', '10/day'."
+  # Rate limiting properties
+  newproperty(:limit, :required_features => :rate_limiting) do
+    desc "Rate limiting value. Example values are: '50/sec', '40/min', 
+          '30/hour', '10/day'."
   end
 
-  newproperty(:burst) do
-    desc "The value for the iptables --limit-burst parameter."
-
-    validate do |value|
-      if value.to_s !~ /^[0-9]+$/
-        self.fail "burst accepts only numeric values"
-      end
-    end
+  newproperty(:burst, :required_features => :rate_limiting) do
+    desc "Rate limiting burst value (per second)."
+    newvalue(/^\d+$/)
   end
   
   validate do
diff --git a/spec/type/iptables_type_spec.rb b/spec/type/iptables_type_spec.rb
index c389cd6..c345440 100644
--- a/spec/type/iptables_type_spec.rb
+++ b/spec/type/iptables_type_spec.rb
@@ -29,7 +29,7 @@ describe Puppet::Type.type(:firewall) do
     end
 
     it 'should fail when the chain value is not recognized' do
-      lambda { @resource[:chain] = 'foo' }.should raise_error(Puppet::Error)
+      lambda { @resource[:chain] = 'not valid' }.should raise_error(Puppet::Error)
     end
   end
 
-- 
2.45.2