Made append_ip_rule() into a wrapper for add_ip_rule(), state tracking bug fixes

- Simplified append_ip_rule() to just be a wrapper around add_ip_rule(),
  which was updated to allow the value "-1" to be passed in as the rule
  insertion number in order to denote "append" (-A <chain>) mode.
- Added "mac_source" tests to t/basic_tests.pl.
- Bug fix to ensure that state tracking arguments are properly processed
  by add_ip_rule().

diff --git a/Changes b/Changes
index a6f54d0..d4a73dd 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,13 @@
 Revision history for Perl extension IPTables::ChainMgr.
 
+1.3 Sun Mar 04 22:45:44 2012
+    - Simplified append_ip_rule() to just be a wrapper around add_ip_rule(),
+      which was updated to allow the value "-1" to be passed in as the rule
+      insertion number in order to denote "append" (-A <chain>) mode.
+    - Added "mac_source" tests to t/basic_tests.pl.
+    - Bug fix to ensure that state tracking arguments are properly processed
+      by add_ip_rule().
+
 1.2 Fri Mar 02 21:09:57 2012
     - Added set_chain_policy() function to allow built-in chain policies to be
       set to the specified target.  iptables/ip6tables does not allow the

diff --git a/lib/IPTables/ChainMgr.pm b/lib/IPTables/ChainMgr.pm
index a5541dc..6f62ce3 100644 (file)
--- a/lib/IPTables/ChainMgr.pm
+++ b/lib/IPTables/ChainMgr.pm
@@ -155,89 +155,17 @@ sub append_ip_rule() {
 
     ### optionally add port numbers and protocols, etc.
     my $extended_href = shift || {};
-    my $iptables = $self->{'_iptables'};
-
-    ### normalize src/dst if necessary; this is because iptables
-    ### always reports the network address for subnets
-    my $normalized_src = $self->normalize_net($src);
-    my $normalized_dst = $self->normalize_net($dst);
-
-    ### first check to see if this rule already exists
-    my ($rule_position, $num_chain_rules)
-            = $self->find_ip_rule($normalized_src, $normalized_dst, $table,
-                $chain, $target, $extended_href);
-
-    if ($rule_position) {
-        my $msg = '';
-        if (keys %$extended_href) {
-            $msg = "Table: $table, chain: $chain, $normalized_src -> " .
-                "$normalized_dst ";
-            for my $key (qw(protocol s_port d_port mac_source)) {
-                $msg .= "$key $extended_href->{$key} "
-                    if defined $extended_href->{$key};
-            }
-            $msg .= 'rule already exists.';
-        } else {
-            $msg = "Table: $table, chain: $chain, $normalized_src -> " .
-                "$normalized_dst rule already exists.";
-        }
-        return 1, [$msg], [];
-    }
-
-    ### we need to add the rule
-    my $ipt_cmd = '';
-    my $msg     = '';
-    my $idx_err = '';
-
-    if (keys %$extended_href) {
-        $ipt_cmd = "$iptables -t $table -A $chain ";
-        $ipt_cmd .= "-p $extended_href->{'protocol'} "
-            if defined $extended_href->{'protocol'};
-        $ipt_cmd .= "-s $normalized_src ";
-        $ipt_cmd .= "--sport $extended_href->{'s_port'} "
-            if defined $extended_href->{'s_port'};
-        $ipt_cmd .= "-d $normalized_dst ";
-        $ipt_cmd .= "--dport $extended_href->{'d_port'} "
-            if defined $extended_href->{'d_port'};
-        $ipt_cmd .= "-m mac --mac-source $extended_href->{'mac_source'} "
-            if defined $extended_href->{'mac_source'};
-        $ipt_cmd .= "-j $target";
-
-        $msg = "Table: $table, chain: $chain, added $normalized_src " .
-            "-> $normalized_dst ";
-        for my $key (qw(protocol s_port d_port mac_source)) {
-            $msg .= "$key $extended_href->{$key} "
-                if defined $extended_href->{$key};
-        }
-
-        ### for NAT
-        if (defined $extended_href->{'to_ip'} and
-                defined $extended_href->{'to_port'}) {
-            $ipt_cmd .= " --to $extended_href->{'to_ip'}:" .
-                "$extended_href->{'to_port'}";
-            $msg .= "$extended_href->{'to_ip'}:$extended_href->{'to_port'}";
-        }
 
-        $msg =~ s/\s*$//;
-    } else {
-        $ipt_cmd = "$iptables -t $table -A $chain " .
-            "-s $normalized_src -d $normalized_dst -j $target";
-        $msg = "Table: $table, chain: $chain, added $normalized_src " .
-            "-> $normalized_dst";
-    }
-    my ($rv, $out_aref, $err_aref) = $self->run_ipt_cmd($ipt_cmd);
-    if ($rv) {
-        push @$out_aref, $msg if $msg;
-    }
-    push @$err_aref, $idx_err if $idx_err;
-    return $rv, $out_aref, $err_aref;
+    ### -1 for append
+    return $self->add_ip_rule($src, $dst, -1, $table,
+        $chain, $target, $extended_href);
 }
 
 sub add_ip_rule() {
     my $self = shift;
     my $src = shift || croak '[-] Must specify a src address/network.';
     my $dst = shift || croak '[-] Must specify a dst address/network.';
-    my $rulenum = shift || croak '[-] Must specify an insert rule number.';
+    (my $rulenum = shift) >= -1 || croak '[-] Must specify an insert rule number, or -1 for append.';
     my $table   = shift || croak '[-] Must specify a table, e.g. "filter".';
     my $chain   = shift || croak '[-] Must specify a chain.';
     my $target  = shift ||
@@ -262,7 +190,7 @@ sub add_ip_rule() {
         if (keys %$extended_href) {
             $msg = "Table: $table, chain: $chain, $normalized_src -> " .
                 "$normalized_dst ";
-            for my $key (qw(protocol s_port d_port mac_source)) {
+            for my $key (qw(protocol s_port d_port mac_source state ctstate)) {
                 $msg .= "$key $extended_href->{$key} "
                     if defined $extended_href->{$key};
             }
@@ -279,18 +207,23 @@ sub add_ip_rule() {
     my $msg     = '';
     my $idx_err = '';
 
-    ### check to see if the insertion index ($rulenum) is too big
-    $rulenum = 1 if $rulenum <= 0;
-    if ($rulenum > $num_chain_rules+1) {
-        $idx_err = "Rule position $rulenum is past end of $chain " .
-            "chain ($num_chain_rules rules), compensating."
-            if $num_chain_rules > 0;
-        $rulenum = $num_chain_rules + 1;
+    if ($rulenum == 0) {
+        $ipt_cmd = "$iptables -t $table -I $chain 1 ";
+    } elsif ($rulenum < 0) {
+        ### switch to append mode
+        $ipt_cmd = "$iptables -t $table -A $chain ";
+    } else {
+        ### check to see if the insertion index ($rulenum) is too big
+        if ($rulenum > $num_chain_rules+1) {
+            $idx_err = "Rule position $rulenum is past end of $chain " .
+                "chain ($num_chain_rules rules), compensating."
+                if $num_chain_rules > 0;
+            $rulenum = $num_chain_rules + 1;
+        }
+        $ipt_cmd = "$iptables -t $table -I $chain $rulenum ";
     }
-    $rulenum = 1 if $rulenum == 0;
 
     if (keys %$extended_href) {
-        $ipt_cmd = "$iptables -t $table -I $chain $rulenum ";
         $ipt_cmd .= "-p $extended_href->{'protocol'} "
             if defined $extended_href->{'protocol'};
         $ipt_cmd .= "-s $normalized_src ";
@@ -309,7 +242,7 @@ sub add_ip_rule() {
 
         $msg = "Table: $table, chain: $chain, added $normalized_src " .
             "-> $normalized_dst ";
-        for my $key (qw(protocol s_port d_port mac_source)) {
+        for my $key (qw(protocol s_port d_port mac_source state ctstate)) {
             $msg .= "$key $extended_href->{$key} "
                 if defined $extended_href->{$key};
         }
@@ -324,8 +257,7 @@ sub add_ip_rule() {
 
         $msg =~ s/\s*$//;
     } else {
-        $ipt_cmd = "$iptables -t $table -I $chain $rulenum " .
-            "-s $normalized_src -d $normalized_dst -j $target";
+        $ipt_cmd .= "-s $normalized_src -d $normalized_dst -j $target";
         $msg = "Table: $table, chain: $chain, added $normalized_src " .
             "-> $normalized_dst";
     }
@@ -366,7 +298,7 @@ sub delete_ip_rule() {
 
     my $extended_msg = '';
     if (keys %$extended_href) {
-        for my $key (qw(protocol s_port d_port mac_source)) {
+        for my $key (qw(protocol s_port d_port mac_source state ctstate)) {
             $extended_msg .= "$key: $extended_href->{$key} "
                 if defined $extended_href->{$key};
         }
@@ -437,6 +369,7 @@ sub find_ip_rule() {
                     d_port
                     to_ip
                     to_port
+                    mac_source
                     state
                     ctstate
                 )) {
@@ -451,6 +384,13 @@ sub find_ip_rule() {
                                     $found = 0;
                                     last;
                                 }
+                            } elsif ($key eq 'mac_source') {
+                                ### make sure case does not matter
+                                unless (lc($extended_href->{$key})
+                                        eq lc($rule_href->{$key})) {
+                                    $found = 0;
+                                    last;
+                                }
                             } else {
                                 unless ($extended_href->{$key}
                                         eq $rule_href->{$key}) {

diff --git a/t/basic_tests.pl b/t/basic_tests.pl
index 2307f51..4dc8cf2 100755 (executable)
--- a/t/basic_tests.pl
+++ b/t/basic_tests.pl
@@ -25,6 +25,8 @@ my $ipv4_dst = '192.168.1.2';
 my $ipv6_src = 'fe80::200:f8ff:fe21:67cf';
 my $ipv6_dst = '0000:0000:00AA:0000:0000:AA00:0000:0001/64';
 
+my $mac_source = 'ff:ff:ff:c6:33:58';
+
 my $logfile   = 'test.log';
 my $PRINT_LEN = 68;
 my $chain_past_end = 1000;
@@ -123,6 +125,7 @@ sub test_cycle() {
     &chain_does_not_exist_test($ipt_obj, $test_table, $test_chain);
     &create_chain_test($ipt_obj, $test_table, $test_chain);
     &add_rules_tests($ipt_obj, $test_table, $test_chain);
+    &find_rules_tests($ipt_obj, $test_table, $test_chain);
     &flush_chain_test($ipt_obj, $test_table, $test_chain);
     &delete_chain_test($ipt_obj, $test_table, $test_jump_from_chain, $test_chain);
 
@@ -233,13 +236,21 @@ sub add_rules_tests() {
     }
 
     for my $target (qw/LOG ACCEPT RETURN/) {
-        &dots_print("add_ip_rules(): $test_table $test_chain $src_ip -> $dst_ip $target ");
+        &dots_print("add_ip_rule(): $test_table $test_chain $src_ip -> $dst_ip $target ");
         my ($rv, $out_ar, $err_ar) = $ipt_obj->add_ip_rule($src_ip,
                 $dst_ip, $chain_past_end, $test_table, $test_chain, $target, {});
 
         &pass_fail($rv, "   Could not add $src_ip -> $dst_ip $target rule.");
     }
 
+    for my $target (qw/LOG ACCEPT RETURN/) {
+        &dots_print("append_ip_rule(): $test_table $test_chain $src_ip -> $dst_ip $target ");
+        my ($rv, $out_ar, $err_ar) = $ipt_obj->add_ip_rule($src_ip,
+                $dst_ip, -1, $test_table, $test_chain, $target, {});
+
+        &pass_fail($rv, "   Could not append $src_ip -> $dst_ip $target rule.");
+    }
+
     return;
 }
 
@@ -299,6 +310,13 @@ sub add_extended_rules_tests() {
                 {'protocol' => 'tcp', 's_port' => 0, 'd_port' => 80, 'ctstate' => 'ESTABLISHED,RELATED'});
         &pass_fail($rv, "   Could not add TCP $src_ip(0) -> $dst_ip(80) ctstate ESTABLISHED,RELATED $target rule");
 
+        ### TCP + mac source
+        &dots_print("add_ip_rules(): $test_table $test_chain TCP $src_ip(0) -> $dst_ip(80) $target mac_source $mac_source ");
+        ($rv, $out_ar, $err_ar) = $ipt_obj->add_ip_rule($src_ip,
+                $dst_ip, $chain_past_end, $test_table, $test_chain, $target,
+                {'protocol' => 'tcp', 's_port' => 0, 'd_port' => 80, 'mac_source' => $mac_source});
+        &pass_fail($rv, "   Could not add TCP $src_ip(0) -> $dst_ip(80) $target mac_source $mac_source");
+
         ### UDP
         &dots_print("add_ip_rules(): $test_table $test_chain UDP $src_ip(0) -> $dst_ip(53) $target ");
         ($rv, $out_ar, $err_ar) = $ipt_obj->add_ip_rule($src_ip,
@@ -343,6 +361,13 @@ sub find_extended_rules_tests() {
                 'd_port' => 80, 'ctstate' => 'ESTABLISHED,RELATED'});
         &pass_fail($rule_position, "   Could not find TCP $src_ip(0) -> $dst_ip(80) ctstate ESTABLISHED,RELATED $target rule");
 
+        &dots_print("find rule: $test_table $test_chain TCP $src_ip(0) -> $dst_ip(80) $target mac_source $mac_source ");
+        ($rule_position, $num_chain_rules) = $ipt_obj->find_ip_rule($src_ip,
+                $dst_ip, $test_table, $test_chain, $target,
+                {'protocol' => 'tcp', 's_port' => 0, 'd_port' => 80,
+                'mac_source' => $mac_source});
+        &pass_fail($rule_position, "   Could not find TCP $src_ip(0) -> $dst_ip(80) $target mac_source $mac_source");
+
         &dots_print("find rule: $test_table $test_chain UDP $src_ip(0) -> $dst_ip(53) $target ");
         ($rule_position, $num_chain_rules) = $ipt_obj->find_ip_rule($src_ip,
                 $dst_ip, $test_table, $test_chain, $target,