From 57a3c2d67ed02f017e8a264c551d9c172b67447f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sat, 18 Nov 2017 01:45:19 +0100 Subject: [PATCH] network: have safe fallback in case of qubes-firewall crash/error When qubes-firewall service is started, modify firewall to have "DROP" policy, so if something goes wrong, no data got leaked. But keep default action "ACCEPT" in case of legitimate service stop, or not starting it at all - because one may choose to not use this service at all. Achieve this by adding "DROP" rule at the end of QBS-FIREWALL chain and keep it there while qubes-firewall service is running. Fixes QubesOS/qubes-issues#3269 --- network/ip6tables | 2 +- network/iptables | 8 ++++---- qubesagent/firewall.py | 10 +++++++--- qubesagent/test_firewall.py | 14 +++++++++----- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/network/ip6tables b/network/ip6tables index 4fd5d90..2b64f6e 100644 --- a/network/ip6tables +++ b/network/ip6tables @@ -1,6 +1,6 @@ # Generated by ip6tables-save v1.4.14 on Tue Sep 25 16:00:20 2012 *filter -:INPUT DROP [1:72] +:INPUT DROP [0:0] :FORWARD DROP [0:0] :OUTPUT ACCEPT [0:0] :QBS-FORWARD - [0:0] diff --git a/network/iptables b/network/iptables index 16f560b..e602dcc 100644 --- a/network/iptables +++ b/network/iptables @@ -1,6 +1,6 @@ # Generated by iptables-save v1.4.5 on Mon Sep 6 08:57:46 2010 *nat -:PREROUTING ACCEPT [85:5912] +:PREROUTING ACCEPT [0:0] :OUTPUT ACCEPT [0:0] :POSTROUTING ACCEPT [0:0] :PR-QBS - [0:0] @@ -14,9 +14,9 @@ COMMIT # Completed on Mon Sep 6 08:57:46 2010 # Generated by iptables-save v1.4.5 on Mon Sep 6 08:57:46 2010 *filter -:INPUT ACCEPT [168:11399] -:FORWARD ACCEPT [0:0] -:OUTPUT ACCEPT [128:12536] +:INPUT DROP [0:0] +:FORWARD DROP [0:0] +:OUTPUT ACCEPT [0:0] :QBS-FORWARD - [0:0] -A INPUT -i vif+ -p udp -m udp --dport 68 -j DROP -A INPUT -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py index ca9a036..c445fc7 100755 --- a/qubesagent/firewall.py +++ b/qubesagent/firewall.py @@ -204,7 +204,7 @@ class IptablesWorker(FirewallWorker): self.run_ipt(family, ['-N', chain]) self.run_ipt(family, - ['-A', 'QBS-FORWARD', '-s', addr, '-j', chain]) + ['-I', 'QBS-FORWARD', '-s', addr, '-j', chain]) self.chains[family].add(chain) def prepare_rules(self, chain, rules, family): @@ -340,8 +340,10 @@ class IptablesWorker(FirewallWorker): # make sure 'QBS_FORWARD' chain exists - should be created before # starting qubes-firewall try: - self.run_ipt(4, ['-nL', 'QBS-FORWARD']) - self.run_ipt(6, ['-nL', 'QBS-FORWARD']) + self.run_ipt(4, ['-F', 'QBS-FORWARD']) + self.run_ipt(4, ['-A', 'QBS-FORWARD', '-j', 'DROP']) + self.run_ipt(6, ['-F', 'QBS-FORWARD']) + self.run_ipt(6, ['-A', 'QBS-FORWARD', '-j', 'DROP']) except subprocess.CalledProcessError: self.log_error('\'QBS-FORWARD\' chain not found, create it first') sys.exit(1) @@ -542,6 +544,8 @@ class NftablesWorker(FirewallWorker): 'table {family} qubes-firewall {{\n' ' chain forward {{\n' ' type filter hook forward priority 0;\n' + ' policy drop;\n' + ' ct state established accept\n' ' }}\n' '}}\n' ) diff --git a/qubesagent/test_firewall.py b/qubesagent/test_firewall.py index f373122..b50fb33 100644 --- a/qubesagent/test_firewall.py +++ b/qubesagent/test_firewall.py @@ -173,7 +173,7 @@ class TestIptablesWorker(TestCase): self.obj.create_chain(addr, chain, family) self.assertEqual(self.obj.called_commands[family], [['-N', chain], - ['-A', 'QBS-FORWARD', '-s', addr, '-j', chain]]) + ['-I', 'QBS-FORWARD', '-s', addr, '-j', chain]]) def test_002_prepare_rules4(self): rules = [ @@ -244,7 +244,7 @@ class TestIptablesWorker(TestCase): self.assertEqual(self.obj.called_commands[4], [ ['-N', chain], - ['-A', 'QBS-FORWARD', '-s', '10.137.0.1', '-j', chain], + ['-I', 'QBS-FORWARD', '-s', '10.137.0.1', '-j', chain], ['-F', chain]]) self.assertEqual(self.obj.loaded_iptables[4], self.obj.prepare_rules(chain, rules, 4)) @@ -258,7 +258,7 @@ class TestIptablesWorker(TestCase): self.assertEqual(self.obj.called_commands[6], [ ['-N', chain], - ['-A', 'QBS-FORWARD', '-s', '2000::a', '-j', chain], + ['-I', 'QBS-FORWARD', '-s', '2000::a', '-j', chain], ['-F', chain]]) self.assertEqual(self.obj.loaded_iptables[6], self.obj.prepare_rules(chain, rules, 6)) @@ -268,9 +268,9 @@ class TestIptablesWorker(TestCase): def test_006_init(self): self.obj.init() self.assertEqual(self.obj.called_commands[4], - [['-nL', 'QBS-FORWARD']]) + [['-F', 'QBS-FORWARD'], ['-A', 'QBS-FORWARD', '-j', 'DROP']]) self.assertEqual(self.obj.called_commands[6], - [['-nL', 'QBS-FORWARD']]) + [['-F', 'QBS-FORWARD'], ['-A', 'QBS-FORWARD', '-j', 'DROP']]) def test_007_cleanup(self): self.obj.init() @@ -429,11 +429,15 @@ class TestNftablesWorker(TestCase): 'table ip qubes-firewall {\n' ' chain forward {\n' ' type filter hook forward priority 0;\n' + ' policy drop;\n' + ' ct state established accept\n' ' }\n' '}\n' 'table ip6 qubes-firewall {\n' ' chain forward {\n' ' type filter hook forward priority 0;\n' + ' policy drop;\n' + ' ct state established accept\n' ' }\n' '}\n' ])