Fix 5247: Firewall groups CLI becomes out of sync with ipset when sets and deletes are contained within a single commit

4 files changed, 82 insertions, 9 deletions

diff --git a/lib/Vyatta/IpTables/IpSet.pm b/lib/Vyatta/IpTables/IpSet.pm
index 20463f1..894bb84 100755
--- a/lib/Vyatta/IpTables/IpSet.pm
+++ b/lib/Vyatta/IpTables/IpSet.pm
@@ -95,7 +95,7 @@ sub exists {
 
     return 1 if   defined $self->{_exists};
     return 0 if ! defined $self->{_name};
-    my $cmd = "ipset -n -L $self->{_name} > /dev/null &>2";
+    my $cmd = "ipset -n -L $self->{_name} > /dev/null 2>&1";
     my $rc = $self->run_cmd($cmd);
     if ($rc eq 0) {
 	$self->{_exists} = 1;
diff --git a/scripts/firewall/vyatta-ipset.pl b/scripts/firewall/vyatta-ipset.pl
index 8614581..4d4cded 100755
--- a/scripts/firewall/vyatta-ipset.pl
+++ b/scripts/firewall/vyatta-ipset.pl
@@ -13,7 +13,7 @@
 # General Public License for more details.
 # 
 # This code was originally developed by Vyatta, Inc.
-# Portions created by Vyatta are Copyright (C) 2009 Vyatta, Inc.
+# Portions created by Vyatta are Copyright (C) 2009-2010 Vyatta, Inc.
 # All Rights Reserved.
 # 
 # Author: Stig Thormodsrud
@@ -158,7 +158,7 @@ sub ipset_copy_set {
         # copy members to new group
         my $tmpfile = "/tmp/set.$$";
         system("ipset -S $set_name > $tmpfile");
-        system("sed -i s/$set_name/$set_copy/g $tmpfile");
+        system("sed -i \'s/ $set_name / $set_copy /g\' $tmpfile");
         system("ipset -R < $tmpfile");
         unlink $tmpfile;
         my $copy  = new Vyatta::IpTables::IpSet($set_copy, $set_type);
diff --git a/templates/firewall/group/address-group/node.tag/address/node.def b/templates/firewall/group/address-group/node.tag/address/node.def
index b0bd955..a04dd5b 100644
--- a/templates/firewall/group/address-group/node.tag/address/node.def
+++ b/templates/firewall/group/address-group/node.tag/address/node.def
@@ -20,22 +20,28 @@ create: tmpgrp=$VAR(../@)-$PPID
         fi
         tmpfile="/tmp/$tmpgrp";
 
+        # echo create $VAR(@) $tmpgrp $COMMIT_SIBLING_POSITION
+
         if [ "$COMMIT_SIBLING_POSITION" = "FIRST" ] || \
            [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
            sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-set-empty \
              --set-name=$VAR(../@) 
            if [ $? != 0 ]; then
+              # echo create $tmpfile;
               touch $tmpfile;
            fi;
            sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=copy-set \
              --set-name=$VAR(../@) --set-type=address --set-copy=$tmpgrp
+           # echo create $tmpgrp
         fi;
 
         sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=add-member \
           --set-name="$tmpgrp" --member="$VAR(@)" --alias=$VAR(../@)
         if [ $? != 0 ]; then
+                # echo error adding, destroy $tmpgrp
                 sudo ipset --destroy $tmpgrp;
                 if [ -e $tmpfile ]; then
+                   # echo destroy $VAR(../@)
                    sudo ipset --destroy $VAR(../@);
                    rm $tmpfile;
                 fi;
@@ -44,12 +50,38 @@ create: tmpgrp=$VAR(../@)-$PPID
 
         if [ "$COMMIT_SIBLING_POSITION" = "LAST" ] || \
            [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
+           # echo swap and destroy $tmpgrp
            sudo ipset --swap $tmpgrp "$VAR(../@)";
            sudo ipset --destroy $tmpgrp;
            rm -f $tmpfile;
         fi;
 
-delete: sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
+delete: tmpgrp=$VAR(../@)-$PPID
+        len=${#tmpgrp}
+        if [ "$len" -gt 31 ]; then
+           tmpgrp=${tmpgrp: -31};
+           if [[ "$tmpgrp" =~ ^- ]]; then
+              tmpgrp=${tmpgrp/-/Z};
+           fi
+        fi
+        tmpfile="/tmp/$tmpgrp";
+
+        # echo delete $VAR(@) $tmpgrp $COMMIT_SIBLING_POSITION
+
+        if [ "$COMMIT_SIBLING_POSITION" = "FIRST" ] || \
+           [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
+           sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-set-empty \
+             --set-name=$VAR(../@) 
+           if [ $? != 0 ]; then
+              # echo create $tmpfile;
+              touch $tmpfile;
+           fi;
+           sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=copy-set \
+             --set-name=$VAR(../@) --set-type=address --set-copy=$tmpgrp
+           # echo create $tmpgrp
+        fi;
+
+        sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
                 --set-name=$VAR(../@) --set-type=address;
         if [ $? == 0 ] ; then
            sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-used \
@@ -61,5 +93,13 @@ delete: sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
         fi
 
         sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=delete-member \
-          --set-name=$VAR(../@) \
+          --set-name=$tmpgrp \
           --member="$VAR(@)"
+
+        if [ "$COMMIT_SIBLING_POSITION" = "LAST" ] || \
+           [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
+           # echo swap and destroy $tmpgrp
+           sudo ipset --swap $tmpgrp "$VAR(../@)";
+           sudo ipset --destroy $tmpgrp;
+           rm -f $tmpfile;
+        fi;
diff --git a/templates/firewall/group/port-group/node.tag/port/node.def b/templates/firewall/group/port-group/node.tag/port/node.def
index f0ab759..6e657c4 100644
--- a/templates/firewall/group/port-group/node.tag/port/node.def
+++ b/templates/firewall/group/port-group/node.tag/port/node.def
@@ -51,8 +51,33 @@ create: tmpgrp=$VAR(../@)-$PPID
            rm -f $tmpfile;
         fi;
 
-delete: sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
-               --set-name=$VAR(../@) --set-type=port;
+delete: tmpgrp=$VAR(../@)-$PPID
+        len=${#tmpgrp}
+        if [ "$len" -gt 31 ]; then
+           tmpgrp=${tmpgrp: -31};
+           if [[ "$tmpgrp" =~ ^- ]]; then
+              tmpgrp=${tmpgrp/-/Z};
+           fi
+        fi
+        tmpfile="/tmp/$tmpgrp";
+
+        # echo delete $VAR(@) $tmpgrp $COMMIT_SIBLING_POSITION
+
+        if [ "$COMMIT_SIBLING_POSITION" = "FIRST" ] || \
+           [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
+           sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-set-empty \
+             --set-name=$VAR(../@) 
+           if [ $? != 0 ]; then
+              # echo create $tmpfile;
+              touch $tmpfile;
+           fi;
+           sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=copy-set \
+             --set-name=$VAR(../@) --set-type=port --set-copy=$tmpgrp
+           # echo create $tmpgrp
+        fi;
+
+        sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
+                --set-name=$VAR(../@) --set-type=port;
         if [ $? == 0 ] ; then
            sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-used \
                --set-name=$VAR(../@) --set-type=port
@@ -61,7 +86,15 @@ delete: sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=is-group-deleted \
               exit 1;
            fi
         fi
- 
+
         sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=delete-member \
-          --set-name=$VAR(../@) \
+          --set-name=$tmpgrp \
           --member="$VAR(@)"
+
+        if [ "$COMMIT_SIBLING_POSITION" = "LAST" ] || \
+           [ "$COMMIT_SIBLING_POSITION" = "FIRSTLAST" ] ; then
+           # echo swap and destroy $tmpgrp
+           sudo ipset --swap $tmpgrp "$VAR(../@)";
+           sudo ipset --destroy $tmpgrp;
+           rm -f $tmpfile;
+        fi;