vyatta-nat: check nat rules for errors before processing them for real

Errors in both source and destination NAT rules can cause either rules
being overwritten or partially, dropped entirely or just end up with
an inconsistent state in comparison to the current configuration.  This
can lead to unpredictable NAT results, which can't even be corrected by
deleting all the nat rules, only a reboot will solve them.

Checking these rules for consistency in a separate loop before they are
applied allows the errors to flagged up and the commit failed before
the nat table is touched.

Bug #493 http://bugzilla.vyos.net/show_bug.cgi?id=493

2 files changed, 34 insertions, 12 deletions

diff --git a/scripts/vyatta-update-dst-nat.pl b/scripts/vyatta-update-dst-nat.pl
index 6543f17..7cf49a2 100755
--- a/scripts/vyatta-update-dst-nat.pl
+++ b/scripts/vyatta-update-dst-nat.pl
@@ -82,6 +82,23 @@ system("$IPTABLES -t nat -L -n >& /dev/null");
 ipt_enable_conntrack('iptables', 'NAT_CONNTRACK');
 
 for $rule (@rule_keys) {
+  my $rule_test = new Vyatta::DstNatRule;
+  $rule_test->setup($CONFIG_LEVEL." rule $rule");
+
+  if (($rules{$rule} eq "static") || ($rules{$rule} eq "deleted")) {
+    next;
+  } else {
+    my ($err, @rule_strs) = $rule_test->rule_str();
+    if (defined $err) {
+      # rule check failed => return error
+      print OUT "Destination NAT configuration error in rule $rule: $err\n";
+      print STDERR "Destination NAT configuration error in rule $rule: $err\n";
+      exit 5;
+    }
+  }
+}
+  
+for $rule (@rule_keys) {
   print OUT "$rule: $rules{$rule}\n";
   my $tmp = `iptables -L -nv --line -t nat`;
   print OUT "iptables before:\n$tmp\n";
@@ -112,12 +129,6 @@ for $rule (@rule_keys) {
   }
   
   my ($err, @rule_strs) = $nrule->rule_str();
-  if (defined $err) {
-    # rule check failed => return error
-    print OUT "Destination NAT configuration error in rule $rule: $err\n";
-    print STDERR "Destination NAT configuration error in rule $rule: $err\n";
-    exit 5;
-  }
   
   if ($rules{$rule} eq "added") {
     foreach my $rule_str (@rule_strs) {
diff --git a/scripts/vyatta-update-src-nat.pl b/scripts/vyatta-update-src-nat.pl
index b908cc1..20f9887 100755
--- a/scripts/vyatta-update-src-nat.pl
+++ b/scripts/vyatta-update-src-nat.pl
@@ -82,6 +82,23 @@ system("$IPTABLES -t nat -L -n >& /dev/null");
 ipt_enable_conntrack('iptables', 'NAT_CONNTRACK');
 
 for $rule (@rule_keys) {
+  my $rule_test = new Vyatta::DstNatRule;
+  $rule_test->setup($CONFIG_LEVEL." rule $rule");
+
+  if (($rules{$rule} eq "static") || ($rules{$rule} eq "deleted")) {
+    next;
+  } else {
+    my ($err, @rule_strs) = $rule_test->rule_str();
+    if (defined $err) {
+      # rule check failed => return error
+      print OUT "Destination NAT configuration error in rule $rule: $err\n";
+      print STDERR "Destination NAT configuration error in rule $rule: $err\n";
+      exit 5;
+    }
+  }
+}
+
+for $rule (@rule_keys) {
   print OUT "$rule: $rules{$rule}\n";
   my $tmp = `iptables -L -nv --line -t nat`;
   print OUT "iptables before:\n$tmp\n";
@@ -112,12 +129,6 @@ for $rule (@rule_keys) {
   }
   
   my ($err, @rule_strs) = $nrule->rule_str();
-  if (defined $err) {
-    # rule check failed => return error
-    print OUT "Source NAT configuration error in rule $rule: $err\n";
-    print STDERR "Source NAT configuration error in rule $rule: $err\n";
-    exit 5;
-  }
   
   if ($rules{$rule} eq "added") {
     foreach my $rule_str (@rule_strs) {