Added some safeguards for invalid firewall rules

No more None dsthosts or port ranges.

fixes QubesOS/qubes-issues#5772

qubesadmin/firewall.py

@@ -86,6 +86,8 @@ def rule(self):
 class DstHost(RuleOption):
     '''Represent host/network address: either IPv4, IPv6, or DNS name'''
     def __init__(self, value, prefixlen=None):
+        if value is None:
+            raise ValueError('DstHost cannot be None')
         # TODO: in python >= 3.3 ipaddress module could be used
         if value.count('/') > 1:
             raise ValueError('Too many /: ' + value)
@@ -158,6 +160,8 @@ def rule(self):
 class DstPorts(RuleOption):
     '''Destination port(s), for TCP/UDP only'''
     def __init__(self, value):
+        if value is None:
+            raise ValueError('Port range cannot be None')
         if isinstance(value, int):
             value = str(value)
         if value.count('-') == 1:
@@ -184,6 +188,8 @@ class IcmpType(RuleOption):
     '''ICMP packet type'''
     def __init__(self, value):
         super(IcmpType, self).__init__(value)
+        if value is None:
+            raise ValueError('ICMP cannot be None')
         value = int(value)
         if value < 0 or value > 255:
             raise ValueError('ICMP type out of range')

qubesadmin/tests/tools/qvm_firewall.py

@@ -96,6 +96,17 @@ def test_006_dsthost_aliases(self):
                 qubesadmin.firewall.Rule(
                     None, action='accept', dsthost='127.0.0.1/32'))
 
+    def test_007_none_errors(self):
+        ns = argparse.Namespace()
+        with self.assertRaises(ValueError):
+            self.action(None, ns, ['dsthost=', 'action=accept'])
+        with self.assertRaises(ValueError):
+            self.action(None, ns, ['dsthost=127.0.0.1', 'dstports=',
+                                   'action=accept'])
+        with self.assertRaises(ValueError):
+            self.action(None, ns, ['dsthost=127.0.0.1', 'icmptype=',
+                                   'action=accept'])
+
 
 class TC_10_qvm_firewall(qubesadmin.tests.QubesTestCase):
     def setUp(self):