From a12e72b89c54db3936762b74f3813d5564a1fd7d Mon Sep 17 00:00:00 2001
From: Pawel Marczewski <pawel@invisiblethingslab.com>
Date: Tue, 14 Jan 2020 10:46:51 +0100
Subject: [PATCH] update_connected_ips: reload nftables using one command

Get rid of race condition between flushing the chains
and adding new rules.
---
 qubesagent/firewall.py      | 47 ++++++++++++++++++-------------------
 qubesagent/test_firewall.py |  3 +--
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py
index 0bf2ed1..0fb73cc 100755
--- a/qubesagent/firewall.py
+++ b/qubesagent/firewall.py
@@ -515,36 +515,35 @@ class NftablesWorker(FirewallWorker):
         family_name = ('ip6' if family == 6 else 'ip')
         table = 'qubes-firewall'
 
-        self.run_nft((
+        nft_input = (
             'flush chain {family_name} {table} prerouting\n'
             'flush chain {family_name} {table} postrouting\n'
-        ).format(family_name=family_name, table=table))
+        ).format(family_name=family_name, table=table)
 
         ips = self.get_connected_ips(family)
-        if not ips:
-            return
+        if ips:
+            addr = '{' + ', '.join(ips) + '}'
+            irule = 'iifname != "vif*" {family_name} saddr {addr} drop\n'.format(
+                family_name=family_name, addr=addr)
+            orule = 'oifname != "vif*" {family_name} daddr {addr} drop\n'.format(
+                family_name=family_name, addr=addr)
 
-        addr = '{' + ', '.join(ips) + '}'
-        irule = 'iifname != "vif*" {family_name} saddr {addr} drop\n'.format(
-            family_name=family_name, addr=addr)
-        orule = 'oifname != "vif*" {family_name} daddr {addr} drop\n'.format(
-            family_name=family_name, addr=addr)
+            nft_input += (
+                'table {family_name} {table} {{\n'
+                '  chain prerouting {{\n'
+                '    {irule}'
+                '  }}\n'
+                '  chain postrouting {{\n'
+                '    {orule}'
+                '  }}\n'
+                '}}\n'
+            ).format(
+                family_name=family_name,
+                table=table,
+                irule=irule,
+                orule=orule,
+            )
 
-        nft_input = (
-            'table {family_name} {table} {{\n'
-            '  chain prerouting {{\n'
-            '    {irule}'
-            '  }}\n'
-            '  chain postrouting {{\n'
-            '    {orule}'
-            '  }}\n'
-            '}}\n'
-        ).format(
-            family_name=family_name,
-            table=table,
-            irule=irule,
-            orule=orule,
-        )
         self.run_nft(nft_input)
 
     def prepare_rules(self, chain, rules, family):
diff --git a/qubesagent/test_firewall.py b/qubesagent/test_firewall.py
index 7de73fc..7ecf0c5 100644
--- a/qubesagent/test_firewall.py
+++ b/qubesagent/test_firewall.py
@@ -553,8 +553,7 @@ class TestNftablesWorker(TestCase):
 
         self.assertEqual(self.obj.loaded_rules, [
             'flush chain ip qubes-firewall prerouting\n'
-            'flush chain ip qubes-firewall postrouting\n',
-
+            'flush chain ip qubes-firewall postrouting\n'
             'table ip qubes-firewall {\n'
             '  chain prerouting {\n'
             '    iifname != "vif*" ip saddr {10.137.0.1, 10.137.0.2} drop\n'