From 5f05bdd480f0de2be2d87934b9b53dab09ead9ea Mon Sep 17 00:00:00 2001
From: Alex Harpin <development@landsofshadow.co.uk>
Date: Sun, 12 Jul 2015 17:17:58 +0100
Subject: 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
---
 scripts/vyatta-update-dst-nat.pl | 23 +++++++++++++++++------
 scripts/vyatta-update-src-nat.pl | 23 +++++++++++++++++------
 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
@@ -81,6 +81,23 @@ system("$IPTABLES -t nat -L -n >& /dev/null");
 # we have some nat rule(s). make sure conntrack is enabled.
 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`;
@@ -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
@@ -81,6 +81,23 @@ system("$IPTABLES -t nat -L -n >& /dev/null");
 # we have some nat rule(s). make sure conntrack is enabled.
 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`;
@@ -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) {
-- 
cgit v1.2.3