From 2c80b1c97aace6a5a3994e0daeb2158878b79b08 Mon Sep 17 00:00:00 2001 From: "Troy J. Farrell" Date: Thu, 13 Mar 2025 11:23:22 -0500 Subject: [PATCH 1/3] firewall: avoid crash on qubesdb long path errors Long hostnames result in qubesdb paths that exceed the current 64 character limit. Instead of crashing the firewall daemon, log the error and continue without adding the hostname to the DNS records. This is a workaround, not a fix, for https://github.com/QubesOS/qubes-issues/9084. The fix requires either longer path support in qubesdb or using a different schema so that keys will not exceed the qubesdb path length limit. --- qubesagent/firewall.py | 13 ++++++++++++- qubesagent/test_firewall.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py index 66407a5ce..8a9910d23 100755 --- a/qubesagent/firewall.py +++ b/qubesagent/firewall.py @@ -167,7 +167,18 @@ def update_dns_info(self, source, dns): self.qdb.rm('/dns/{}/'.format(source)) for host, hostaddrs in dns.items(): - self.qdb.write('/dns/{}/{}'.format(source, host), str(hostaddrs)) + path = '/dns/{}/{}'.format(source, host) + try: + self.qdb.write(path, str(hostaddrs)) + except Exception as err: + if len(path) > 64 and err.args == (0, 'Error'): + self.log.error(('Unable to add DNS information for {} ({})' + ' due to qubesdb path length limit').format( + host, source)) + self.log.error('See https://github.com/QubesOS/' + 'qubes-issues/9085') + else: + raise def update_handled(self, addr): """ diff --git a/qubesagent/test_firewall.py b/qubesagent/test_firewall.py index 4938d4516..3ed4a8dc9 100644 --- a/qubesagent/test_firewall.py +++ b/qubesagent/test_firewall.py @@ -40,6 +40,8 @@ def rm(self, path): self.entries.pop(path) def write(self, path, val): + if len(path) > 64: + raise DummyQubesDBError(0, 'Error') self.entries[path] = val def multiread(self, prefix): @@ -65,6 +67,8 @@ def read_watch(self): except IndexError: return None +class DummyQubesDBError(Exception): + "Raised by QubesDB" class FirewallWorker(qubesagent.firewall.FirewallWorker): def __init__(self): @@ -159,6 +163,35 @@ def test_701_dns_info(self): self.obj.apply_rules('10.137.0.1', [{'action': 'drop'}]) self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) + def test_701_dns_info(self): + self.obj.conntrack_get_connections = Mock(return_value=[]) + rules = [ + {'action': 'accept', 'proto': 'tcp', + 'dstports': '80-80', 'dsthost': 'ripe.net'}, + {'action': 'drop'}, + ] + self.obj.apply_rules('10.137.0.1', rules) + self.assertIsNotNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) + self.obj.apply_rules('10.137.0.1', [{'action': 'drop'}]) + self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) + + def test_702_dns_info_qubesdb_path_length_crash(self): + self.obj.conntrack_get_connections = Mock(return_value=[]) + rules = [ + {'action': 'accept', 'proto': 'tcp', + 'dstports': '443-443', 'dsthost': 'www.google.com'}, + {'action': 'accept', 'proto': 'tcp', + 'dstports': '443-443', 'dsthost': 'prod-dynamite-prod-05-us-signaler-pa.clients6.google.com'}, + {'action': 'drop'}, + ] + self.obj.apply_rules('10.137.0.22', rules) + self.assertIsNotNone(self.obj.qdb.read('/dns/10.137.0.22/www.google.com')) + # Unfortunately, this is assertIsNone until the QubesDB path length limit is raised. + self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.22/prod-dynamite-prod-05-us-signaler-pa.clients6.google.com')) + self.obj.apply_rules('10.137.0.22', [{'action': 'drop'}]) + self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.22/www.google.com')) + self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.22/prod-dynamite-prod-05-us-signaler-pa.clients6.google.com')) + class TestNftablesWorker(TestCase, WorkerCommon): def setUp(self): super(TestNftablesWorker, self).setUp() From 86fecb9eeb40fd6254a85ce19e00a3414c1a62b9 Mon Sep 17 00:00:00 2001 From: "Troy J. Farrell" Date: Thu, 13 Mar 2025 11:33:35 -0500 Subject: [PATCH 2/3] Remove accidentally duplicated test function --- qubesagent/test_firewall.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/qubesagent/test_firewall.py b/qubesagent/test_firewall.py index 3ed4a8dc9..98106d30d 100644 --- a/qubesagent/test_firewall.py +++ b/qubesagent/test_firewall.py @@ -163,18 +163,6 @@ def test_701_dns_info(self): self.obj.apply_rules('10.137.0.1', [{'action': 'drop'}]) self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) - def test_701_dns_info(self): - self.obj.conntrack_get_connections = Mock(return_value=[]) - rules = [ - {'action': 'accept', 'proto': 'tcp', - 'dstports': '80-80', 'dsthost': 'ripe.net'}, - {'action': 'drop'}, - ] - self.obj.apply_rules('10.137.0.1', rules) - self.assertIsNotNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) - self.obj.apply_rules('10.137.0.1', [{'action': 'drop'}]) - self.assertIsNone(self.obj.qdb.read('/dns/10.137.0.1/ripe.net')) - def test_702_dns_info_qubesdb_path_length_crash(self): self.obj.conntrack_get_connections = Mock(return_value=[]) rules = [ From 7518028a189389bd4913f32827af4d6f563a1b4a Mon Sep 17 00:00:00 2001 From: "Troy J. Farrell" Date: Thu, 13 Mar 2025 12:29:50 -0500 Subject: [PATCH 3/3] Fix the issue URL in the log message --- qubesagent/firewall.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qubesagent/firewall.py b/qubesagent/firewall.py index 8a9910d23..9ffc5f9d3 100755 --- a/qubesagent/firewall.py +++ b/qubesagent/firewall.py @@ -176,7 +176,7 @@ def update_dns_info(self, source, dns): ' due to qubesdb path length limit').format( host, source)) self.log.error('See https://github.com/QubesOS/' - 'qubes-issues/9085') + 'qubes-issues/issues/9084') else: raise