Skip to content

Commit 8619266

Browse files
committed
firewall blocks signal during read operations
1 parent 0a67a77 commit 8619266

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

qubesagent/firewall.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,9 @@ def dns_addresses(family=None):
344344
def main(self):
345345
self.terminate_requested = False
346346
self.reload_requested = False
347+
# Block SIGHUP and SIGTERM during all qdb operations to prevent interrupting request-response pairs which corrupts protocol state.
348+
# Signals are only unblocked during read_watch() which is safe to interrupt as it's just waiting, not mid-operation.
349+
signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGHUP, signal.SIGTERM})
347350
self.init()
348351
self.run_firewall_dir()
349352
if not self.is_custom_persist_enabled():
@@ -366,11 +369,22 @@ def main(self):
366369
self.handle_addr(source_addr)
367370
self.reload_requested = False
368371
self.sd_notify('READY=1')
372+
# Unblock signals only during read_watch()
373+
signal.pthread_sigmask(signal.SIG_UNBLOCK, {signal.SIGHUP, signal.SIGTERM})
374+
375+
# Re-check flags after unblocking, in case signal arrived
376+
if self.terminate_requested or self.reload_requested:
377+
signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGHUP, signal.SIGTERM})
378+
continue
369379
try:
370380
watch_path = self.qdb.read_watch()
371381
except OSError: # EINTR
372-
# signal received, re-check loop condition
382+
# signal received, block signals again and re-check loop condition
383+
signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGHUP, signal.SIGTERM})
373384
continue
385+
386+
#Block signals again before doing any qdb work
387+
signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGHUP, signal.SIGTERM})
374388

375389
if watch_path is None:
376390
break

qubesagent/test_firewall.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,3 +572,49 @@ def test_is_blocked(self):
572572

573573
for server in dns_servers_ipv6:
574574
self.assertTrue(self.obj.is_blocked(rules, ("udp", server, "53"), dns))
575+
576+
def test_main_blocks_signals_during_qdb_operations(self):
577+
#Test that signals are blocked during qdb operations and only unblocked during read_watch().
578+
579+
self.obj.qdb.entries['/qubes-firewall/10.137.0.1/policy'] = b'accept'
580+
self.obj.qdb.entries['/connected-ips'] = b''
581+
self.obj.qdb.entries['/connected-ips6'] = b''
582+
583+
# Use signal module from firewall to avoid adding new import
584+
sig = qubesagent.firewall.signal
585+
586+
# Track sigmask calls
587+
sigmask_calls = []
588+
original_sigmask = sig.pthread_sigmask
589+
590+
def mock_sigmask(how, mask):
591+
sigmask_calls.append((how, mask))
592+
return original_sigmask(how, set()) # Don't actually block
593+
594+
# Make read_watch() terminate the loop after first call
595+
call_count = [0]
596+
def mock_read_watch():
597+
call_count[0] += 1
598+
if call_count[0] == 1:
599+
return '/qubes-firewall/10.137.0.1'
600+
self.obj.terminate_requested = True
601+
raise OSError("Interrupted")
602+
603+
self.obj.qdb.read_watch = mock_read_watch
604+
605+
with patch.object(sig, 'pthread_sigmask', mock_sigmask):
606+
self.obj.main()
607+
608+
# Verify signal blocking pattern:
609+
# 1. SIG_BLOCK at start
610+
# 2. SIG_UNBLOCK before read_watch
611+
# 3. SIG_BLOCK after read_watch (or in except)
612+
613+
block_calls = [c for c in sigmask_calls if c[0] == sig.SIG_BLOCK]
614+
unblock_calls = [c for c in sigmask_calls if c[0] == sig.SIG_UNBLOCK]
615+
616+
self.assertGreater(len(block_calls), 0, "Should have SIG_BLOCK calls")
617+
self.assertGreater(len(unblock_calls), 0, "Should have SIG_UNBLOCK calls")
618+
# First call should be SIG_BLOCK
619+
self.assertEqual(sigmask_calls[0][0], sig.SIG_BLOCK)
620+

0 commit comments

Comments
 (0)