From 84b4345897825efd8f2ac6144f16a501f1b157e5 Mon Sep 17 00:00:00 2001
From: Alex Harpin <development@landsofshadow.co.uk>
Date: Sun, 19 Jul 2015 10:18:30 +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
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
---
 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..de60d16 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..53cf39d 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::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`;
@@ -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