Repository navigation
fix(minvmd): drop a deny-all box's dns queries outside the zone host-side - #2014
Merged
Merged
Conversation
…side On a VM-backed host the host-side egress gate is the deciding copy of NET-141's deny-all case for own-address rows. A deny-all row's resolver carve-out now carries only a standard query whose every question is an A lookup of a box-zone name, the one thing the switch's resolver answers without asking upstream. Every other datagram to DNS's port is dropped before it is written on to the switch: a name outside the zone, a zone name asked for any other type, a multi-question query hiding either behind a zone A, and anything that does not parse as a standard query. The drop is silent toward the guest and says so once per box per name per interval through the gate's limiter. The predicate lives on BoxRecord and is derived at registration from the host-registered egress policy, never from a guest report. Host-address boxes ride the node row, which is never deny-all, and allow-list rows are untouched.
Contributor
|
Warning Review limit reached
This review includes 3 billable files and costs up to $0.75.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 59 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 5 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughChangesDeny-all DNS handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant EgressRelay
participant deny_all_refusal
participant BoxRecord
participant DropLimiter
EgressRelay->>deny_all_refusal: Check UDP DNS datagram
deny_all_refusal->>BoxRecord: Check registered deny-all status
deny_all_refusal-->>EgressRelay: Return refusal or no refusal
alt Query refused
EgressRelay->>DropLimiter: Rate-limit warning
else Query accepted
EgressRelay->>EgressRelay: Continue forwarding and pin observation
end
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable issue is established for the DNS gate; it is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
A rabbit checks each name at night, Comment |
The switch's resolver answers a name locally only on an exact, case-sensitive `.min.internal.` suffix and forwards everything else upstream, the apex `min.internal` itself and any other casing of the apex labels included. The deny-all gate matched the zone case-insensitively and admitted the apex, so such a query passed the gate and was forwarded. A deny-all zone name now sits strictly below the apex and ends in the raw wire labels `min` and `internal`, byte for byte in lowercase. The labels below the apex may carry any case.
norrietaylor
marked this pull request as ready for review
October 6, 2026 07:09
Contributor
|
Approved: no findings at Nothing to do. Clobber: none: |
Member
Author
|
ARCH PASS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On a VM-backed host, minvmd's host-side egress gate now silently drops a DNS datagram from an own-address deny-all row unless every question in it is an A lookup of a box-zone name. This is the deciding copy, outside the VM escape boundary. The in-VM relay gate (draft #2013) is the precision copy that honest clients hit first and that answers REFUSED. This PR does not depend on #2013's code. It reuses the same predicate semantics.
Requirements
BoxRecordcomes from host-registered egress. Zone names pass.The architecture review added two items to this scope, both covered here:
Design
BoxRecord::is_deny_all()(box_registry.rs): adeny_allfield, computed once inBoxRegistry::registerfromregistration.egress. It is true when everyallow_*list is present and empty, the same predicate as minimald'sclassifier::admits_nothing. The field is registration-frozen and never reads the row's guest-filled runtime half. A row with no egress declaration is never deny-all. That includes the node namespace row, which carries every host-address box's frames, so host-address boxes are out of scope by construction.dns_pins::deny_all_refusal(table, pkt, datagram): a pure decision covering UDP frames to port 53 from a deny-all row. It returnsSome(refusal), naming the row, the first failing name and its type, when any of these holds:Every question is checked. A zone name here follows gvproxy's own test, not the zone's. gvproxy v0.8.9 (
pkg/services/dns/dns.go) treats a name as local only on an exact, case-sensitive.min.internal.suffix and never lowercases, so the gate follows the same rule:minandinternal, compared as raw label bytes.x.MIN.INTERNAL,x.Min.Internalandx.min.INTERNALare dropped, because gvproxy forwards them upstream. Labels below the apex may carry any case, soX.min.internalpasses.min.internalitself upstream, so an A query for it is dropped.evil\.minunderinternal), which a plain string-suffix match would accept.Hook (
egress_gate.rsrelay_frames_to_switch): after the frame verdict admits the frame and before the write, the gate parses the UDP port-53 datagram once. Ifdeny_all_refusalsays drop, the gate callslimiter.warn_deny_all_queryand doescontinue, so the frame is never written and nothing is sent back.observe_querynow reuses the same parsed datagram after the write.Warn line:
DropLimiter::warn_deny_all_queryuses the existing refusal key (box, rule, name) and the bounded table with its flood fallback. The rule isegress-deny-all-dns-query. Fields:source,namespace,name,query_type.TCP 53: the frame verdict already drops TCP to the resolver for a deny-all row, because the NET-079 carve-out is UDP-only. The relay test asserts this.
Tests
net::dns_pins::tests::deny_all_row_passes_only_zone_a_lookups:web.min.internal,host.min.internal, andX.min.internal/HOST.min.internal(case below the apex);x.MIN.INTERNAL,x.Min.Internal,x.min.INTERNAL,HOST.Min.Internal, the apexmin.internalandMIN.INTERNAL(names built withName::from_asciiso the case reaches the wire);x.min.internalandhost.min.internal;net::dns_pins::tests::non_deny_all_rows_are_not_decided: allow-list rows, no-policy rows, closed-names-open-subnets rows, the node namespace and unregistered sources are never decided here.net::egress_gate::tests::deny_all_row_query_outside_zone_dropped_host_side, at relay level:source,namespaceandname;host.min.internalincluded;box_registry::tests::deny_all_predicate_reads_the_host_registration_only: the predicate is set only by the deny-all shape. A guest runtime port report neither lifts it nor sets it.continueremoved, the relay test fails.Residuals
host.min.internalresolves) runs through the in-VM gate.admits_nothing. minimald's copy is inclassifier.rsand minvmd's is inbox_registry.rs. A follow-up moves the predicate ontosessions::EgressPolicybefore the deny-all DNS work closes.evil\.min.internalpasses it. The host copy here matches by label and drops it. Worth aligning in fix(minimald): refuse a deny-all box's names outside the zone at its relay #2013.Validation
macOS arm64, minvmd scope:
just fix: clean.just test:Summary [6.172s] 1007 tests run: 1007 passed, 19 skipped.just clippy: no warnings.just clippy-strict:clippy-strict: clean.What CI proves
The macOS lane and the Linux lanes build and test minvmd, which runs the new unit and relay-level tests. Nothing here touches minimald or a VM path, and no e2e leg was added.
Summary by CodeRabbit
min.internalsubdomains.