qubes-firewall crashed parsing conntrack output

[coyotebush]

Qubes OS release

Qubes OS 4.2

Brief summary

Yesterday I was editing some firewall rules, then later noticed that newly launched qubes were unable to reach the internet. I discovered that the qubes-firewall service had crashed.

Steps to reproduce

1. Update firewall rules in qube settings
2. Probably some unlucky IO timing?

Expected behavior

Firewall keeps updating as rules are changed and new qubes are started.

Actual behavior

qubes-firewall crashed: found in sys-firewall journalctl -u qubes-firewall.service

Feb 07 14:52:47 sys-firewall python3[570]: detected unhandled Python exception in '/usr/bin/qubes-firewall'
Feb 07 14:52:47 sys-firewall qubes-firewall[570]: Traceback (most recent call last):
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/bin/qubes-firewall", line 5, in <module>
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     sys.exit(main())
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:              ^^^^^^
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 638, in main
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     worker.main()
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 353, in main
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     self.handle_addr(source_addr)
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 295, in handle_addr
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     self.apply_rules(addr, rules)
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 598, in apply_rules
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     self.apply_rules_family(source, rules, 4)
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 590, in apply_rules_family
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     is_blocked = self.is_blocked(rules, con, dns)
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:   File "/usr/lib/python3.12/site-packages/qubesagent/firewall.py", line 254, in is_blocked
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:     if int(con_dport) != 53 or not con_dst in dns_servers:
Feb 07 14:52:47 sys-firewall qubes-firewall[570]:        ^^^^^^^^^^^^^^
Feb 07 14:52:47 sys-firewall qubes-firewall[570]: ValueError: invalid literal for int() with base 10: '5conntrack'
Feb 07 14:52:47 sys-firewall systemd[1]: qubes-firewall.service: Main process exited, code=exited, status=1/FAILURE
Feb 07 14:52:47 sys-firewall systemd[1]: qubes-firewall.service: Failed with result 'exit-code'.

Additional information

If I followed the code correctly, it seems like qubes-firewall must have read dport=5conntrack in the output of conntrack. The only place "conntrack" normally appears in the output of conntrack is in the message conntrack v1.4.7 (conntrack-tools): ... printed to stderr before exiting. So that message probably ended up interleaved with normal output such as dport=53.

Two things look suspicious to me, and I'd be happy to change them if desired:

1. stderr=subprocess.STDOUT combines stderr with stdout - but that risks interleaving like this, and probably nothing printed to stderr is usefully parseable anyway.
2. The int() call has no ValueError handling - although a better handling isn't entirely obvious.

[marmarek]

stderr=subprocess.STDOUT combines stderr with stdout - but that risks interleaving like this, and probably nothing printed to stderr is usefully parseable anyway.

Indeed this looks to be a bad idea here...

[marmarek]

On a more generic approach, that service could use automatic restart on failure

[Eric678]

The universal qubes fix!

I just noticed this as my qubes-firewall table in sys-firewall stopped updating a week or so ago - been twiddling nftables manually since - not knowing how it was supposed to work. int() got 4conntrack in my case...

[coyotebush]

Two users in a week or so is less rare than I feared! I can send a PR this week for at least the stderr thing.

I first restarted just the service (systemctl restart qubes-firewall.service) after it crashed, and one effect I noticed is that the contents of nft list chain qubes-firewall forward ended up repeated. So automatic restarts could cause that chain to grow quite long, unless the service is also changed to clear the chain when it starts before appending to it.

[coyotebush]

For interest: I started by logging lines containing "conntrack" in conntrack_get_connections. Given intersecting connections and rules, the line printed was reliably a combination of an output line and the status line (containing sport=591conntrack, src=conntrack, secctx=system_u:oconntrack, etc.)