Merge pull request #14 from zdc/T1456

[ipset] T1456: Add check for duplicate items in port-group before commit

3 files changed, 63 insertions, 3 deletions

diff --git a/lib/Vyatta/IpTables/IpSet.pm b/lib/Vyatta/IpTables/IpSet.pm
index 276b845..d7a014a 100755
--- a/lib/Vyatta/IpTables/IpSet.pm
+++ b/lib/Vyatta/IpTables/IpSet.pm
@@ -410,11 +410,25 @@ sub check_member {
 sub member_exists {
     my ($self, $member) = @_;
 
-    my $cmd = "ipset -T $self->{_name} $member -q";
-    my $rc = $self->run_cmd($cmd);
-    return $rc ? 0 : 1;
+    # check if a member is a port range and roll through all members it is
+    if ($member =~ /([\d]+)-([\d]+)/) {
+        foreach my $port ($1..$2) {
+            # test port with ipset
+            my $cmd = "ipset -T $self->{_name} $port -q";
+            my $rc = $self->run_cmd($cmd);
+            # return true if port was found
+            return 1 if !$rc;
+        }
+        # return false if ports was not found in set
+        return 0;
+    } else {
+        my $cmd = "ipset -T $self->{_name} $member -q";
+        my $rc = $self->run_cmd($cmd);
+        return $rc ? 0 : 1;
+    }
 }
 
+
 sub add_member {
     my ($self, $member, $alias, $hyphenated_port) = @_;
 
diff --git a/scripts/firewall/vyatta-ipset.pl b/scripts/firewall/vyatta-ipset.pl
index 65e0325..0a079f3 100755
--- a/scripts/firewall/vyatta-ipset.pl
+++ b/scripts/firewall/vyatta-ipset.pl
@@ -384,6 +384,47 @@ sub show_port_groups {
     print "$group\n";
   }
 }
+
+# check set in configuration for duplicate elements
+sub check_duplicates {
+  my ($set_name, $set_type, $set_family) = @_;
+  my $cfg = new Vyatta::Config;
+  my $cpath = ($set_family eq 'inet') ? "firewall group $set_type-group $set_name" : "firewall group ipv6-$set_type-group $set_name";
+
+  # get items array
+  my @vals = $cfg->returnValues("$cpath $set_type");
+
+  # check duplicates in port-group
+  if ($set_type eq "port") {
+    # define hash with ports as keys
+    my %portlist;
+
+    for my $item (@vals) {
+      # check if this is a port range
+      if ($item =~ /([\d]+)-([\d]+)/) {
+        foreach my $port ($1..$2) {
+          return "Port $port exist in more than one item\n" if (exists $portlist{$port});
+          $portlist{$port} = undef;
+        }
+
+      # check if this is an alphabetic port name
+      } elsif ($item =~ /^\D+/) {
+        my $port = getservbyname($item, "");
+        return "Port $port exist in more than one item\n" if (exists $portlist{$port});
+        $portlist{$port} = undef;
+
+      # process simple numeric ports
+      } else {
+        return "Port $item exist in more than one item\n" if (exists $portlist{$item});
+        $portlist{$item} = undef;
+      }
+    }
+  }
+
+  # do not return anything if there are no duplicates
+  return;
+}
+
 #
 # main
 #
@@ -437,6 +478,8 @@ $rc = ipset_is_group_defined($set_name, $set_type, $set_family) if $action eq 'i
 $rc = update_set($set_name, $set_type, $set_family) if $action eq 'update-set';
 $rc = prune_deleted_sets() if $action eq 'prune-deleted-sets';
 
+$rc = check_duplicates($set_name, $set_type, $set_family) if $action eq 'check-duplicates';
+
 if (defined $rc) {
     print $rc;
     exit 1;
diff --git a/templates/firewall/group/port-group/node.def b/templates/firewall/group/port-group/node.def
index 729165f..7f37cb4 100644
--- a/templates/firewall/group/port-group/node.def
+++ b/templates/firewall/group/port-group/node.def
@@ -15,6 +15,9 @@ syntax:expression: pattern $VAR(@) "^[^!]" ; \
 syntax:expression: pattern $VAR(@) "^[^|;&$<>]*$" ; \
        "Firewall group name cannot contain shell punctuation"
 
+commit:expression:exec "/opt/vyatta/sbin/vyatta-ipset.pl --action=check-duplicates --set-name=$VAR(@) --set-type=port --set-family=inet"; \
+       "There are duplicates inside port-group $VAR(@)"
+
 end: if sudo /opt/vyatta/sbin/vyatta-ipset.pl --action=update-set \
         --set-name="$VAR(@)" --set-type=port --set-family=inet; then
         ${vyatta_sbindir}/vyatta-firewall-trap.pl --level="firewall group port-group $VAR(@)"