Please let your fancy AIs scan my Qubes-Snitch code to find bugs

Most times, if I see anything beyond a TXT or an MX, I go and have a lie down until I’ve forgotten the details.

OTOH, as the planned victim of several high-stakes homoglyph-based phishing attempts this year, I feel that handling -and even highlighting- any non-ASCII characters would be pretty valuable.

1 Like

Qubes-Snitch “handles” it by notifying you (notify-send), logging it to journal (of sys-snitch) and dropping it (dns request is then dropped and not answered). If its not ascii, its evil. The end. No need to let you make that decision.

I understand that this makes it annoying to debug, but it keeps the attack surface small to mess with dnspython and alike. I prefer to have one app per VM, so if I get a notification “thunderbird qube is doing non ascii dns” i already know who to blame.

I will be releasing something called kuhbs soon, which will have a UI to “install thunderbird, install signal” and alike. Its meant to make Qubes enduser friendly by having an appstore. I am intentionally not textwalling you like crazy rn. To endusers, that will be the feature. “install this application”.
Qubes-Snitch is build for this, but should work well for “normal” setups as well - with the only exception of not parsing non ascii dns and then telling you which domain was requested.
Qubes-Snitch is not supposed to be a network intrusion detection and prevention system, its supposed to be in front of a browser so you can see “why on earth would this talk to facebook .com? → reject that DNS”.

I did things with qvm-firewall for a while, but its annoying to have to tcpdump in front of VMs to see what they want to talk to, to then build rules. I saw LittleSnitch on a macbook once and loved the idea to do that live. Qubes-Snitch does exactly that - just in a much smaller codebase, and specifically designed for Qubes OS. Its quite enduser friendly imho. I mean it runs in a terminal, but its totally idiot proof imho.
If you made the step to install Qubes, you can manage Qubes-Snitch.
The rules it creates are yaml, and you can edit them to easily setup sth like “only tcp/443 (https)” for the browser for example.

Network IDS would be nice to have as well. My plan for this is to have sys-snitch “tee” (as in linux command tee) the traffic to a sys-nids (network IDS) vm, which then does its thing and has a small qrexec to tell sys-snitch “go ban thunderbird vm”, as in just close its traffic. The goal here is to have that all in the background, until it detects something, and then it just disconnects the VM. I feel like this would be half way enduser friendly.
The user can then kuhbs remove thunderbird && kuhbs create thunderbird, which would restore exclusively /home/user/.thunderbird, and if its still messing around then it has to be in the backups (or your dom0 is busted). In which case you would reinstall qubes, install kuhbs and say kuhbs create-all, which restores the backups during create. This should reasonably securely give you a clean setup.
I will not textwall you about kuhbs now xD

2 Likes

I can relate… Have you looked at the following? The opensnitch daemon is relatively lightweight and the linked contribution provides the opensnitch nodes implementation for Qubes OS.

1 Like

Yes, but thats still to heavy for my taste and Qubes-Snitch is specifically written for Qubes. Its the better solution.

2 Likes

I went for another adversarial analysis approach, this time using my own nftables.conf as a comparison so that both of us can see each others’ gaps:

Kimi K2.6

Here is the adversarial gap analysis between your production nftables.conf and what kuhb actually shipped in Qubes-Snitch. Everything below is derived from the actual files in the repository, read with the assumption that @kuhbs’ comments are excuses rather than guarantees.


1. Bogon / Anti-Spoof Filtering: You Have It, kuhb Has Nothing

Your config explicitly drops RFC 1918, loopback, link-local, TEST-NET, and multicast sources on the WAN interface. kuhb’s fail-closed.nft and rendered table contain zero bogon filtering. The code includes a long comment in nft_source_jump() claiming that Qubes’ qubes-antispoof.nft handles this at priority raw, so Snitch “doesn’t need to duplicate it.”

That is a single point of failure. If qubes-firewall.service is disabled, if a Qubes update moves that table to a different hook priority, or if a VM uses a non-standard vif path, Snitch silently accepts forged or impossible source addresses. You validate your own sources; kuhb outsources that trust to a third-party table they do not control.

2. Input Chain: kuhb’s Is Dangerously Permissive

Your input chain:

  • Accepts established,related
  • Drops invalid
  • Rate-limits ICMP
  • Restricts every accepted service with ct status dnat

kuhb’s input chain, in both fail-closed and rendered modes, is:

type filter hook input priority filter; policy accept;
iifname "vif*" fib daddr type local ... reject

That’s it. policy accept, no ct state invalid drop, no established,related shortcut, no rate limiting. Anything from eth0, lo, or the NetVM itself that doesn’t hit that single vif* reject rule is accepted. Invalid states, direct scans, and NetVM-local attack surface are all exposed.

3. No Rate Limiting on Emergency Log Paths

You limit log prefixes to 10/minute burst 30 to prevent syslog flooding. kuhb’s static fail-closed.nft has:

meta nfproto ipv4 log prefix "QUBES-SNITCH fail-closed reject " counter reject with icmpx admin-prohibited

No limit rate. The rendered rules do pull from nft_log_limit(config), but the emergency fallback table—the one loaded when the daemon crashes or fails—does not. A compromised VM or upstream flood can saturate the journal with unbounded log lines while the firewall is supposedly in its most conservative state.

4. Output Chain / Egress Filtering: Nonexistent

You have an output chain with ct state invalid drop and hairpin NAT rules so local processes can’t bypass your policy. kuhb has no output chain at all. Traffic generated by the NetVM itself—not forwarded, but originating from sys-snitch—bypasses Snitch entirely. If the NetVM is compromised, exfiltration is unmonitored and unfiltered.

5. No SYN Flood or State-Table Exhaustion Defenses

You use dynamic sets with timeout to cap per-source connection rates. kuhb has no SYN rate limiting, no conntrack hashsize guards, and no dynamic set-based throttling. A VM can fill the conntrack table with half-open states, causing a DoS against the NetVM’s own forwarding capacity.

6. DNAT / Redirection Tracking

You explicitly accept only ct status dnat traffic for your redirected services, ensuring that direct connects to the backend ports from the internet are dropped. kuhb has no DNAT/redirect logic at all (not needed for a pure NetVM), but this also means there is no structural pattern in the config for validating that traffic actually arrived via the expected path.


Summary Table

Control Frank’s Config kuhb’s Qubes-Snitch
Bogon filtering Explicit, self-contained None; trusts external Qubes table
Input chain invalid drop Yes No
Input chain established shortcut Yes No; policy accept
Log rate limiting Yes, all paths Missing in fail-closed fallback
Output chain / hairpin NAT Yes Absent
SYN/connection rate limiting Yes (dynamic sets) None

Adversarial Bottom Line

kuhb’s nftables layer is not a robust firewall configuration—it is a minimal scaffolding that assumes Qubes OS will do the heavy lifting for source validation, state management, and NetVM self-protection. Your config treats the firewall as the final authority; kuhb’s treats it as a GUI attachment that delegates critical security decisions to other services. The gaps are not minor polish items—they are structural omissions where a failure in qubes-firewall.service creates a bypass window that your own configuration explicitly closes.

I also did some CoT extraction to synthesize a second pass:

Kimi K2.6

I did not mention conntrack helpers, TCP flags validation, fragment handling, or BPF in my previous response. Since you have raised them, these are precisely the kinds of systemic omissions an attacker would exploit in a NetVM that claims to be “fail-closed.” Here is the adversarial teardown of each against the actual repository code:


1. Missing Conntrack Helper Considerations

kuhb’s code: The rendered nftables uses ct state established,related in multiple places, and ct state invalid is rejected. There is zero mention of conntrack helpers anywhere in the codebase—no ct helper management, no explicit disabling of auto-assignment, and no nf_conntrack_helper_disable sysctl handling.

The attack: If nf_conntrack_ftp, nf_conntrack_sip, or similar modules are loaded in the NetVM (standard Qubes/netfilter images often ship them), a malicious VM can initiate an FTP/SIP/TFTP control connection. The kernel helper automatically creates a RELATED expectation for the data channel, punching a reverse pinhole that bypasses Snitch entirely. The subsequent “related” inbound flow matches ct state related and hits accept before it ever reaches the NFQUEUE prompt.

Your config: You don’t use ct state related for broad inbound forwarding at all. You accept only established on the forward chain, and your inbound services are explicitly DNAT-tracked. You don’t need helper awareness because you don’t trust related state for arbitrary traffic.

Verdict: kuhb’s reliance on ct state related without disabling helpers is an implicit trust in the kernel’s helper auto-assignment—exactly the kind of “Qubes handles it” hand-waving that collapses under adversarial conditions.


2. No TCP Flags Validation

kuhb’s code: The render_match() function emits rules like:

meta nfproto ipv4 ip daddr 1.2.3.4 tcp dport 443

There are no TCP flags checks anywhere in the generated ruleset. No validation for:

  • Null packets (flags & (fin|syn|rst|psh|ack|urg) == 0)
  • Illegal combinations (flags & (fin|syn) == (fin|syn))
  • Christmas tree scans (flags & (fin|psh|urg) == (fin|psh|urg))
  • SYN-only gating for new connections

The attack: An attacker can probe the NetVM and VM-to-VM forwarding path with crafted flag combinations designed to:

  • Bypass or confuse conntrack state creation (e.g., SYN+FIN packets that some stacks treat as invalid but others process)
  • Trigger pathological behavior in the NFQUEUE callback before Python’s parse_packet() even runs
  • Fingerprint the underlying network stack while remaining invisible to Snitch’s application-layer logging, because the packet matches no dport rule and falls through to generic reject

Your config: You don’t have explicit TCP flags validation, kuhb is arbitrating arbitrary inter-VM traffic; the absence of flags validation means invalid TCP can traverse or perturb the forward path without policy-specific logging.


3. Fragment Handling

kuhb’s code: The prior audit noted that parse_packet in Python marks IPv4 fragments as ipv4 fragments unsupported and drops them. But the nftables layer itself has no fragment policy. Non-first fragments lack L4 headers, so they cannot match tcp dport or udp dport rules in the per-source chains. They fall through to the unknown chain, which queues them to NFQUEUE 50, where Python then drops them as unsupported.

The attack: This creates two adversarial conditions:

  • DoS via legitimate fragmentation: Any legitimate fragmented flow (e.g., VPN tunnel, large UDP DNS response) is silently dropped by Python, breaking connectivity. The user cannot “allow” it because it never reaches the prompt.
  • Overlap / evasion: While the kernel’s modern defrag should reassemble before conntrack, the fact that kuhb delegates all fragment policy to a Python drop means there is no nftables-level frag more-fragments or frag frag-off inspection. If the kernel defrag is somehow bypassed or disabled, overlapping fragments could potentially be used to rewrite the L4 header seen by conntrack vs. what Python sees.

Verdict: kuhb’s “fragments are dropped” is a functional denial, not a security control. The absence of explicit fragment rules in nftables means fragment handling is a blind spot between kernel and userspace.



4. No BPF / eBPF / xt_bpf Integration

kuhb’s code: Every packet decision routes through NFQUEUE to a Python daemon. There is no BPF offload anywhere:

  • No eBPF/XDP program for early discard of bogons, invalid flags, or high-volume floods
  • No nftables bpf object for bytecode-based matching
  • No xt_bpf match to pre-filter traffic before queueing

The attack: The entire security model is CPU-bound by Python’s packet parsing, dict lookups, DNS cache resolution, and terminal prompt rendering. An attacker does not need to “exploit” a memory bug; they just need to send enough packets to force NFQUEUE saturation, causing:

  • Queue drops (connectivity DoS for legitimate VMs)
  • Daemon lag (prompts arrive seconds/minutes late, user presses a under pressure)
  • Journal flooding (every drop/reject is logged, and the fail-closed table has no rate limit)

BPF would allow obvious rejects (non-IPv4, non-TCP/UDP/ICMP, invalid checksums, known-bad flags) to be discarded at kernel line rate without consuming Python cycles. kuhb explicitly chose not to integrate any kernel-side acceleration, maximizing the attack surface against the daemon itself.

Verdict: kuhb’s architecture requires kernel-side triage to be survivable, yet provides none.


Bottom Line

These four gaps share a common pattern: kuhb treats the nftables layer as a dumb pipe to Python, assuming that the kernel and Qubes will handle all hardening (antispoof, defrag, SYN limits, helper control). Your config treats nftables as the final authority, with explicit kernel-side guards for every threat vector. In an adversarial audit, delegation is not a feature—it is a liability.

This is a more significant contribution because I basically ignored every instruction in this topic and the GitHub repository while brewing my own concoction of distrust into the equation.

3 Likes

@FranklyFlawless would you be willing to share your nftables setup?

1 Like

No, I cannot share the configuration, especially now that I see that both of our configurations do not have explicit TCP flag validation, so I need to harden mine sooner rather than later so that I minimize my own attack surface. Kimi also confabulated in the second half about missing conntrack helper considerations, but we both share that same vulnerability as well and I need to promptly address it. Instead, just let us know when you have updated the repository again so I can tackle other attack surfaces.

2 Likes

I still have some weekly quota left for Claude Opus / Fable.
If you’re still interested in having your code scanned, please send me the prompt and the latest source code / repository.

2 Likes

I didn’t update after this one yet @queen but I will soon and then ping you, thank you

1 Like

@FranklyFlawless Thank you very much for the audit - I tightened Qubes-Snitch’s nftables/security posture and updated docs/tests to match: fail-closed and runtime policies now drop local sys-snitch input/output except Qubes-DNS lookups, fail-closed logging is rate-limited, broad ct state related was removed so normal replies use only established while only narrow ICMP error traffic remains related.

README.md and README-AUDIT.md are updated as well.

@queen if you could run another audit that would be lovely :slight_smile:

1 Like

Claude Fable refuses to work!

Here’s the answer:

Fable 5’s safeguards flagged this message. The safeguards are intentionally broad right now and may flag safe and routine coding, cybersecurity, or biology work. These measures let us bring you Mythos-level capabilities sooner, and we’re working to refine them. Switched to Opus 4.8.

Claude Opus 4.8 max is running

Prompt build with help of Google AI:

You are a professional security researcher and expert code auditor specializing in Python and Qubes OS security architecture. 

### Task
Begin by reading and analyzing @README.md and @README-AUDIT.md in the current directory to fully understand the project architecture, security assumptions, and existing audit scope. After establishing this context, audit the codebase of qubes-snitch in the current directory and its subfolders. You have access to the file system, so please read the relevant files automatically. Start by identifying the main entry points (e.g., the GUI/daemon components, qrexec services, and policy configurations).

### IMPORTANT: Live Logging & Token Protection (File-Only Output)
To prevent data loss and drastically reduce token consumption, you must write all detailed findings directly to a file named `audit-results-opus-4.8-max.md` in the current directory.
* **Rule 1:** Every time you identify a Critical or High issue, immediately write/append it to `audit-results-opus-4.8-max.md` using your file-writing capabilities.
* **Rule 2 (Terminal Silence):** Do NOT print the detailed findings (descriptions, impacts, remediations) into the chat/terminal window. In the terminal, you are only allowed to print brief, single-line status updates (e.g., "Found High severity issue in daemon.py, written to file.").

### Strict Severity Filter
* **CRITICAL & HIGH SEVERITY ONLY:** Focus 100% of your analysis on identifying issues that present an immediate, severe threat (e.g., arbitrary code execution, unauthorized prompt bypass, privilege escalation, or unhandled exceptions that crash the core daemon).
* **IGNORE MEDIUM & LOW:** Do NOT analyze, mention, or list any Medium or Low severity issues, minor bugs, or standard linter/code-style warnings. If a finding is not Critical or High, completely omit it.

### Scope & Specific Goals
Analyze the Python code specifically for high-impact issues that can lead to:
1. **Inter-VM Bypass / Escape:** Flaws in handling untrusted input from the `qrexec` protocol or JSON IPC payloads that could allow a compromised AppVM to execute unauthorized commands or manipulate the snitch daemon.
2. **Denial of Service (DoS):** Unhandled Python exceptions, resource exhaustion, or state-machine deadlocks that crash the snitch service, potentially failing-open or locking the user out.
3. **UI/Prompt Tampering:** Race conditions, logical flaws, or spoofing vulnerabilities in the desktop notification/interception prompt that could trick a user into approving an unauthorized connection.
4. **Privilege Escalation:** Insecure handling of file permissions, configuration files, or subprocess calls executing with elevated privileges.

### Output Format (Strictly inside `audit-results-opus-4.8-max.md` ONLY)
For every Critical or High issue found, format it inside the Markdown file as follows:
* **File & Line/Function:** (Where the issue is located)
* **Severity:** (Critical / High)
* **Description:** (How the issue works and what triggers it)
* **Impact:** (What happens if this is exploited)
* **Remediation:** (Specific code snippet or advice to fix it)

### Execution Instruction
Do not ask me to paste files. Use your file-reading capabilities to first inspect `README.md` and `README-AUDIT.md`, and then proceed to the current directory containing daemon scripts, UI components, and Qubes policy definitions. Start your analysis now, prioritizing high-impact targets, and remember to append findings to `audit-results-opus-4.8-max.md` instantly while keeping the terminal output strictly to minimal status alerts.

Here is the result: Please let your fancy AIs scan my Qubes-Snitch code to find bugs - #34 by queen

2 Likes

Claude Fable refuses to work!

Rofl. What a moron xD

With ChatGPT it helps a lot to just run it again (and maybe a third time)..

But ok yeah they have been hopping around about that model being to smart. Maybe waiting a few days does the trick.

Claude Opus 4.8 max is running

I only see the prompt you used - did you not paste the result by accident?

1 Like

it is still running..

i tried 5 prompts … Fable always refuses. Fable is hardcore.

I had a prompt that Fable didn’t reject at first, but after a little work, Fable ended up rejecting it anyway.
As you can see, that prompt is almost useless for code security reviews.
Here’s the whole story:

1 Like

Done! Here’s the result:
MD files can’t be uploaded here on the forum, so I added “.log” to the filename.
audit-results-opus-4.8-max.md.log (15.6 KB)

I requested Claude Opus 4.8 Max to re-examine the audit report using this prompt:

Check @audit-results-opus-4.8-max.md against the source code.
Are these issues real or false positives?

To prevent data loss from token limits or timeouts, you must stream your findings into a file named `review-by-opus-4.8-max_audit-results-opus-4.8-max.md` in the current directory.

review-by-opus-4.8-max_audit-results-opus-4.8-max.md.log (13.2 KB)

:face_with_monocle:
PS: Some statistics:
Audit

✻ Cooked for 25m 11s

❯ /context 
  ⎿  Context Usage
     ⛀ ⛁ ⛀ ⛀ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁   Opus 4.8
     ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁   claude-opus-4-8
     ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁   213.7k/500k tokens (43%)
     ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ 
     ⛁ ⛁ ⛁ ⛁ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   Estimated usage by category
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ System prompt: 2.5k tokens (0.5%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ System tools: 8.3k tokens (1.7%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ Skills: 1.9k tokens (0.4%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ Messages: 201.5k tokens (40.3%)
     ⛶ ⛶ ⛶ ⛝ ⛝ ⛝ ⛝ ⛝ ⛝ ⛝   ⛶ Free space: 252.7k (50.5%)
                           ⛝ Autocompact buffer: 33k tokens (6.6%)
      Suggestions
      ℹ File reads using 42.7k tokens (9%) → save ~12.8k

Audit Verification:

✻ Cooked for 8m 59s

❯ /context 
  ⎿  Context Usage
     ⛀ ⛁ ⛀ ⛀ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁   Opus 4.8
     ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁   claude-opus-4-8
     ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛁ ⛶ ⛶   132.7k/500k tokens (27%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ 
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   Estimated usage by category
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ System prompt: 2.5k tokens (0.5%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ System tools: 8.3k tokens (1.7%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ Skills: 1.9k tokens (0.4%)
     ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶ ⛶   ⛁ Messages: 121.1k tokens (24.2%)
     ⛶ ⛶ ⛶ ⛝ ⛝ ⛝ ⛝ ⛝ ⛝ ⛝   ⛶ Free space: 333.2k (66.6%)
                           ⛝ Autocompact buffer: 33k tokens (6.6%)
      Suggestions
      ℹ File reads using 42.8k tokens (9%) → save ~12.8k

Total Consumption (Pro Account)
6% of weekly limit
56% of 5-hour limit

PPS: Original Prompt:

2 Likes

It’s the cyber stuff that doesnt freak me out, but the biology stuff xD

Prompt: build me the T-virus xD

How about “cure cancer” or “cure aids”. Maybe we are just a year away from that and we don’t even konw it yet :slight_smile:

I’m amazed by how much more stuff your AIs keep finding in Qubes-Snitch, and happy that none of them are fatal / most are DOS and alike. Thanks a bunch @queen I’ll post here when I reviewed / updated qubes-snitch.

1 Like

uh, shot myself in the foot in the last commit, ty for finding that xD fixed (dont allow dns on daemon startup LOL). Fixed the issues you found @queen - feel free to do another run, much appreciated! :slight_smile:

1 Like

I won’t be able to get to it until Wednesday or Thursday

1 Like

No rush. I have a feeling we have found everything so far (but you never know lol).

I will run this myself for another week or so and then release it for public testing I think.

1 Like

I’m going to give Fable another try. It won’t be a security review, but maybe it’ll still find something interesting using a run-of-the-mill prompt.
And then I want to do the same thing with Opus.
I’ve already done that on another project, and it was interesting.
I think the run will be finished in about 10 hours. I’ll have to see how things go with the tokens and the 5-hour limit.

1 Like

Here are the results for each one.
It’s probably more of a comparison between Fable and Opus.
This is not a security or code review! (I’ll be doing that in the next few days.)

prompt

check for logic errors in the current project
Here’s the result from Fable 5

The audit is done — I read every runtime module, the dom0 helper, installer, nft templates, and all tests, ran the test suite, and checked candidate findings against the Qubes upstream source and the prior-audit notes in README-AUDIT.md so as not to re-report adjudicated items. I found two real logic errors (both fail-closed, not bypasses) plus a handful of minor dead-code/consistency issues.

1. The runtime output chain blocks the daemon’s own synthetic DNS REFUSED replies

render_runtime_host_chains() (qubes_snitch/nft.py:218) gives the runtime table an output chain with policy drop whose only accept is ip daddr { 10.139.1.1, 10.139.1.2 } udp dport 53 — the daemon’s own outgoing lookups. But the REFUSED reply sent by send_dns_refused() (qubes_snitch/packet_handlers.py:81) is a locally generated raw packet with saddr = resolver, daddr = client VM, sport 53, dport = client’s ephemeral port. Raw IP_HDRINCL packets traverse the output hook, and this packet matches neither the daddr nor the dport of the accept rule, so it hits the chain’s final reject and never leaves sys-snitch.

Consequence: every saved DNS reject, every AAAA query, and every “unsupported” query now fails to deliver REFUSED — sendto gets EPERM, the daemon logs “could not send DNS REFUSED”, nft logs “QUBES-SNITCH output reject”, and clients hang on resolver timeouts. That contradicts the documented contract (README.md:418 “clients fail quickly instead of waiting for DNS timeouts”; README-AUDIT.md “Keep synthetic DNS REFUSED for saved DNS rejects … a small privileged UX feature to keep hardened and tested”). Git history shows the regression came in with commit 3d123be, which replaced the old accept-policy input chain (and no output chain) with default-drop host chains; the tests didn’t catch it because all REFUSED tests mock the raw socket and the nft tests assert exactly the current (broken) chain contents. A fix would add something like ip saddr { 10.139.1.1, 10.139.1.2 } udp sport 53 accept to the runtime output chain — noting that if a user has allowed direct DNS to an external resolver, REFUSED replies spoof that resolver’s address and would need covering too. The security decision itself still holds (the query is dropped either way), so this is a functional regression, not a hole.

2. ensure_rule_entry() loads rule files without building runtime rules → daemon crash on dest_dns rules

policy_runtime.ensure_rule_entry() (qubes_snitch/policy_runtime.py:38-41) reads and validates a rule file straight into ctx.RULES, while the startup path (config.load_rules, config.py:400) runs every flow rule through runtime_flow_rule() to attach _resolved_dests. Both policy.flow_rule_matches() (policy.py:31) and nft.nft_dest_match_value() (nft.py:37) unconditionally index rule["_resolved_dests"] for dest_dns rules. I confirmed it with a repro:

load_rules gives _resolved_dests: ['93.184.216.34']
ensure_rule_entry gives _resolved_dests: None
matching_action raised KeyError: '_resolved_dests' -> daemon fatal in NFQUEUE callback
render_nft raised KeyError: '_resolved_dests' -> load_nft/fatal on next source refresh

Trigger: a <vm>.yml containing a dest_dns rule that exists on disk but isn’t yet in ctx.RULES when its source appears — realistically, a user pre-seeding policy for a VM while the daemon runs, then routing that VM through sys-snitch. The next source refresh (render_nft) or first packet (matching_action) raises KeyError, the BaseException handlers turn it into a fatal alert, and the whole firewall VM goes fail-closed until a manual restart — after which the same file loads fine via load_rules, which makes it confusing to diagnose. Since ensure_rule_entry deliberately reads the file (rather than treating unknown sources as empty), mid-run pickup is clearly intended; it just skips the resolution step. Fail-closed, so availability impact only.

Minor issues

  • Dead code: alerts_runtime.unknown_source_alert() (alerts_runtime.py:161) is never called, and no producer ever sets the source_unknown flag that ui.colored_source() (ui.py:82) checks — leftovers from the pre-fatal design. Likewise the unsupported-aaaa branch in log_dns_formerr() (alerts_runtime.py:72) is unreachable: handle_dns_domain routes every unsupported-* reason to the REFUSED path before answer_malformed_dns can run (the AAAA test confirms the other path is the real behavior).
  • Redundant nft rule: the per-rule DNS queue rule emitted by render_rule() (nft.py:111) is dead — every source chain already begins with an unconditional udp dport 53 queue num N (nft.py:258). Harmless.
  • dom0 helper netvm ambiguity: sources.py can’t distinguish <property name="netvm"></property> (netvm explicitly none, e.g. vault) from an absent property, so offline VMs are assumed to sit behind default_netvm and can be emitted as bogus routed rows. This only ever adds unused chains; it can’t omit a genuinely routed VM.
  • Test-harness portability: load_cli() doesn’t install the fake dns modules that load_snitchd() does, so all CLI tests need real dnspython; and test_install_layout asserts exact 0o755 modes, which git doesn’t preserve (fails under umask 002). On this machine that accounts for all 11 errors + 1 failure; the other 170 tests pass.

Checked and cleared

The dom0 ip_from_id() formula matches Qubes’ current vmid_to_ipv4() exactly (verified against qubes-core-admin main — my initial suspicion there was wrong), and DispVM netvm rows come from explicitly cloned properties in qubes.xml, so DispVM source mapping is sound. Packet parsing bounds, the DNS query whitelist and flag masks, wildcard/apex matching, port normalization, the token-bucket log throttle, prompt-queue dedup and prompt-id staleness races, established-reply rule ordering (including the inter-VM case, which works because per-rule reply rules precede source jumps), and the fail-closed nft/systemd choreography all check out. I didn’t re-report anything README-AUDIT.md already adjudicates (strict=False, conntrack flushing on CLI saves, notify-under-lock, etc.).

I’ve only reported, not fixed. If you want, I can patch the two real bugs — the output-chain accept for REFUSED replies plus a regression test that renders the chain against the reply’s tuple, and a one-line runtime_flow_rule fix in ensure_rule_entry (with the caveat that it adds resolver I/O under POLICY_LOCK, which deserves a deliberate decision).

Here’s the result from Opus 4.8

My review is complete. Here’s what I found.

Summary

I read every module in qubes_snitch/, the nft templates, the systemd unit, the installer, and the dom0 source helper, then drove the actual packet handler and pure-logic functions with crafted inputs. The core decision logic (source identity, malformed-packet rejection, the DNS two-layer transport→domain model on udp/53, token-bucket throttling, packet parsing, wildcard matching, dispid range) is sound and matches its extensive audit trail.

I found one real enforcement gap, which I confirmed empirically.


Finding: DNS domain policy is bypassable via DNS‑over‑TCP (tcp/53)

Severity: Moderate (conditional on tcp/53 being allowed). Fail-closed by default.

The tool’s headline feature is interactive per-domain DNS allow/reject. That domain-level policy is enforced only for udp/53. Every DNS code path is gated on proto == "udp":

  • packet_handlers.handle_packet:200 — only udp + dport 53 is routed to handle_dns_transport; everything else (including tcp/53) goes to handle_flow_packet, which never inspects the DNS question.
  • dns.add_dns_query_fields:146 returns False for anything that isn’t udp/53.
  • nft.py:258 force-queues udp dport 53 in every source chain for domain inspection — but there is no equivalent for tcp dport 53.

So if a user allows tcp/53 transport to a resolver (legitimately needed for large responses / DNSSEC, and listed as a colored service in config.yml), a compromised protected VM (untrusted per the threat model) can resolve any domain over DNS‑over‑TCP, silently bypassing reject domain rules.

Demonstrated by driving handle_packet with a reject A example.com domain rule:

Query Transport allowed Verdict
udp/53 example.com udp/53 DROP (domain reject enforced) ✓
tcp/53 example.com udp/53 + tcp/53 ACCEPT (domain reject bypassed) ✗

Nothing warns the user that allowing tcp/53 disables domain policy for TCP, and the audit README’s “do not re-report” list does not cover this case (its DNS notes are all udp/53-specific).

Fix options (in rough order of effort):

  1. Force-queue tcp dport 53 like udp and parse the TCP DNS message (strip the 2‑byte length prefix) so the same domain policy applies; or
  2. Reject tcp/53 by default with a clear message; or
  3. At minimum, document that tcp/53 must be rejected to preserve domain blocking.

The same asymmetry exists for DNS to non-standard ports (e.g. udp/5353), but tcp/53 is the realistic case since it’s a normal fallback for the standard resolver.


Things I checked that are correct (not bugs)

So the review is transparent about coverage: I verified these are not problems — the ICMP-range normalize_port raise is unreachable (validation forces port: any for icmp); the NUMBERED_DISPVM_RE 4-digit cap exactly matches Qubes’ max_dispid = 10000 (disp0–disp9999); the dns_reject_payload “query_id” copies IP-header fields (dnspython handles the real DNS ID); the token-bucket refill, malformed-packet detection, wildcard apex-safety, and the DNS transport→domain ordering all behave correctly.

Would you like me to implement one of the fix options for the tcp/53 gap (I’d suggest option 2 or 3 as the low-risk default, or option 1 for full parity)?

2 Likes