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

Errors in both source and destination NAT rules can cause either rules
to be overwritten (completely or partially), dropped entirely, or just
ending 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
correct the issue.

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..de60d16 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..53cf39d 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::SrcNatRule;
+  $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 "Source NAT configuration error in rule $rule: $err\n";
+      print STDERR "Source 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) {