From 27b96cce4c2f58d0539b1bca9e22b765b8149391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 16 Oct 2017 01:47:20 +0200 Subject: [PATCH] firewall: skip expired rules Expired rules are skipped while loading the firewall. Do that also when such rules expired after loading the firewall. This applies to both Admin API and actually applying the rules (sending them to appropriate VM). Related QubesOS/qubes-issues#3020 --- qubes/api/admin.py | 3 ++- qubes/firewall.py | 4 ++++ qubes/tests/api_admin.py | 5 ++++- qubes/tests/firewall.py | 4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/qubes/api/admin.py b/qubes/api/admin.py index 2a0f00c4..a5e78f72 100644 --- a/qubes/api/admin.py +++ b/qubes/api/admin.py @@ -1131,7 +1131,8 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI): self.fire_event_for_permission() return ''.join('{}\n'.format(rule.api_rule) - for rule in self.dest.firewall.rules) + for rule in self.dest.firewall.rules + if rule.api_rule is not None) @qubes.api.method('admin.vm.firewall.Set', scope='local', write=True) diff --git a/qubes/firewall.py b/qubes/firewall.py index c3a3922b..4ed16149 100644 --- a/qubes/firewall.py +++ b/qubes/firewall.py @@ -349,6 +349,8 @@ class Rule(qubes.PropertyHolder): @property def api_rule(self): values = [] + if self.expire and self.expire.expired: + return None # put comment at the end for prop in sorted(self.property_list(), key=(lambda p: p.__name__ == 'comment')): @@ -592,6 +594,8 @@ class Firewall(object): if addr_family is not None: exclude_dsttype = 'dst4' if addr_family == 6 else 'dst6' for ruleno, rule in zip(itertools.count(), self.rules): + if rule.expire and rule.expire.expired: + continue # exclude rules for another address family if rule.dsthost and rule.dsthost.type == exclude_dsttype: continue diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index a99f01f3..72d3ab86 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1736,8 +1736,11 @@ class TC_00_VMs(AdminAPITestCase): dstports='1-1024'), qubes.firewall.Rule(action='drop', proto='icmp', comment='No ICMP'), + # should not output expired rule qubes.firewall.Rule(action='drop', proto='udp', expire='1499450306'), + qubes.firewall.Rule(action='drop', proto='udp', + expire='2099450306'), qubes.firewall.Rule(action='accept'), ] value = self.call_mgmt_func(b'admin.vm.firewall.Get', @@ -1745,7 +1748,7 @@ class TC_00_VMs(AdminAPITestCase): self.assertEqual(value, 'action=accept proto=tcp dstports=1-1024\n' 'action=drop proto=icmp comment=No ICMP\n' - 'action=drop expire=1499450306 proto=udp\n' + 'action=drop expire=2099450306 proto=udp\n' 'action=accept\n') self.assertFalse(self.vm.firewall.save.called) self.assertFalse(self.app.save.called) diff --git a/qubes/tests/firewall.py b/qubes/tests/firewall.py index 155164c5..3875cafe 100644 --- a/qubes/tests/firewall.py +++ b/qubes/tests/firewall.py @@ -468,7 +468,7 @@ class TC_08_Rule(qubes.tests.QubesTestCase): self.assertEqual(rule.api_rule, rule_txt) def test_009_from_api_string(self): - rule_txt = 'action=accept expire=1463292452 proto=tcp ' \ + rule_txt = 'action=accept expire=2063292452 proto=tcp ' \ 'comment=Some comment, with spaces' with self.assertNotRaises(ValueError): rule = qubes.firewall.Rule.from_api_string( @@ -476,7 +476,7 @@ class TC_08_Rule(qubes.tests.QubesTestCase): self.assertEqual(rule.comment, 'Some comment, with spaces') self.assertEqual(rule.proto, 'tcp') self.assertEqual(rule.action, 'accept') - self.assertEqual(rule.expire, '1463292452') + self.assertEqual(rule.expire, '2063292452') self.assertIsNone(rule.dstports) self.assertIsNone(rule.dsthost) self.assertEqual(rule.api_rule, rule_txt)