From 14bed3ad112362ecf9fabc0bd5d5ecbeef96dd0d Mon Sep 17 00:00:00 2001
From: Alex Harpin <development@landsofshadow.co.uk>
Date: Sat, 28 Nov 2015 20:00:46 +0000
Subject: vyatta-cfg-firewall: check rules for errors before processing them

Errors in firewall 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 firewall results, which can't even be
corrected by deleting all the firewall rules, only a reboot or manual
intervention 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 iptables are touched.

Bug #623 http://bugzilla.vyos.net/show_bug.cgi?id=623
---
 scripts/firewall/vyatta-firewall.pl | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/scripts/firewall/vyatta-firewall.pl b/scripts/firewall/vyatta-firewall.pl
index 0f855ba..c2727cc 100755
--- a/scripts/firewall/vyatta-firewall.pl
+++ b/scripts/firewall/vyatta-firewall.pl
@@ -523,6 +523,45 @@ sub update_rules {
     my $policy_set = 0;
     log_msg "update_rules: [$name] = [$nodes{$name}], policy [$policy] log [$policy_log]";
 
+    $config->setLevel("$tree $name rule");
+    my %test_rule_hash = $config->listNodeStatus();
+
+    foreach my $test_rule (sort numerically keys %test_rule_hash) {
+        if ("$test_rule_hash{$test_rule}" eq 'static') {
+            next;
+        } elsif ("$test_rule_hash{$test_rule}" eq 'added') {
+            my $test_node = new Vyatta::IpTables::Rule;
+            $test_node->setup("$tree $name rule $test_rule");
+            $test_node->set_ip_version($ip_version_hash{$tree});
+            my ($err_str, @rule_strs) = $test_node->rule();
+            if (defined($err_str)) {
+                Vyatta::Config::outputError([$tree,$name],"Firewall configuration error: $err_str\n");
+                exit 1;
+            }
+            my $test_chain = chain_configured(2, $name, $tree);
+            if (defined($test_chain)) {
+                # Chain name must be unique in both trees
+                Vyatta::Config::outputError([$tree,$name], "Firewall configuration error: Rule set name \"$name\" already used in \"$test_chain\"\n");
+                exit 1;
+            }
+        } elsif ("$test_rule_hash{$test_rule}" eq 'changed') {
+            my $test_node = new Vyatta::IpTables::Rule;
+            $test_node->setup("$tree $name rule $test_rule");
+            $test_node->set_ip_version($ip_version_hash{$tree});
+            my ($err_str, @rule_strs) = $test_node->rule();
+            if (defined($err_str)) {
+                Vyatta::Config::outputError([$tree,$name],"Firewall configuration error: $err_str\n");
+                exit 1;
+            }
+        } elsif ("$test_rule_hash{$test_rule}" eq 'deleted') {
+            if (Vyatta::IpTables::Mgr::chain_referenced($table, $name, $iptables_cmd)) {
+                # Disallow deleting a chain if it's still referenced
+                Vyatta::Config::outputError([$tree,$name],"Firewall configuration error: Cannot delete rule set \"$name\" (still in use)\n");
+                exit 1;
+            }
+        }
+    }
+
     if ($nodes{$name} eq 'static') {
 
         # not changed. check if stateful.
-- 
cgit v1.2.3