#129 enforce the relayed synthetic answer so _gateway can be a rule - #131
Merged
Merged
Conversation
#128 relayed systemd-resolved's synthetic names to the stub as resolution only: the answer fed neither hostname tracking nor the firewall, so a runner whose host agent sits at the default gateway showed a block on a bare IP and could only be allowed by a provider CIDR. The relayed answer now goes through enforceDNSResponse exactly as an upstream one, still uncached: an allow rule naming _gateway opens the gateway's address on its ports, a deny closes it, and with no rule the connection is attributed to _gateway rather than a bare IP. The agent already accepted underscore rule values; the companion change in app admits them at the control plane. Verified live in Lima under enforce: without a rule, curl to the gateway logs "Connection blocked dst=_gateway"; with the rule, "DNS resolution: added to firewall hostname=_gateway action=allow" and "Connection allowed dst=_gateway". Signed-off-by: Matthew DeVenny <matt@codecargo.com>
There was a problem hiding this comment.
Pull request overview
This pull request enforces relayed synthetic DNS answers, enabling _gateway hostname rules and improving connection attribution.
Changes:
- Applies normal enforcement to synthetic DNS responses.
- Adds allow/deny enforcement tests.
- Documents underscore labels and synthetic-name behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
README.md |
Documents underscore labels and _gateway rules. |
pkg/dns/synthetic.go |
Enforces relayed synthetic answers. |
pkg/dns/synthetic_test.go |
Tests tracking and firewall behavior. |
design.md |
Updates synthetic-name enforcement documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…acked Review on #131: syntheticQuery accepts any label under .localhost, and hostnameIPs has no eviction, so routing every relayed answer through enforcement let a job grow tracking without bound by issuing unique localhost aliases — no allow rule needed. Enforcement now applies to the four underscore names only, a fixed set. The localhost family is relayed as in #128 and never tracked: loopback is auto-allowed, and attributing 127.0.0.1 to an alias buys nothing. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
Review on #131, two findings. The machine's own hostname is a synthetic record too — its configured name, its first label, and the mDNS <label>.local form — and /etc/hosts does not always pin it: on the Lima box getent for the hostname failed under the redirect and sudo warned "unable to resolve host" on every invocation. Seeded at Start and relayed like the underscore set; three names at most. _gateway is an address alias, never a name a peer presents, but enforceDNSResponse also mints L7 forward-resolution evidence and an all-ports allow then L7-scopes the address for TLS, HTTP and QUIC — so with --tls-sni on, HTTP to the host agent carrying "Host: <ip>" would have been an L7 miss. The evidence line now skips names the proxy answered from resolved's local state (localAlias: the underscore set and the machine names). SCOPE IFF BOUND makes that sufficient — registerL7 scopes nothing without evidence — so the L7 matcher learns nothing about synthetic names and the shared function gains no parameter. L4 enforcement and attribution are unchanged; the rule is L4-only, like the CIDR it replaces. Pinned by AllowDoesNotL7Scope, with the wire-path contrast (evidence minted, all-ports scope) pinned on the host-search end-to-end test. syntheticServer returns two values again; the one test that needs the mock asserts s.firewall. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
Review on #131: localAlias re-derived inside enforceDNSResponse a fact serveSynthetic already had, from the same two slices — the skip flag by another name, with two sites that had to agree. The shared function now takes wireIdentity: the upstream answer and the CNAME pre-resolve pass true and mint L7 forward-resolution evidence; the synthetic relay passes false and mints none. enforceDNSResponse knows nothing of synthetic names. Classification is one method, lookupSynthetic (name, expanded, enforce, ok): the localhost family relays only; the underscore set enforces, with its search-expanded form answered NXDOMAIN; the machine's own names enforce with no search expansion, since a real host's expanded form is the name that resolves — runner.lan stays on the ordinary path, pinned by test. seedMachineNames deduplicates, so a hostname already in the mDNS form yields two names, not three. No behaviour change; the live gateway, machine-hostname and sudo checks repeat unchanged. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
jordanjennings
requested changes
Sep 16, 2026
jordanjennings
left a comment
jordanjennings
left a comment
There was a problem hiding this comment.
Blocking on two issues: the startup pre-population path defeats the wireIdentity=false invariant this PR introduces (pkg/dns/synthetic.go:136), and the machine-name seeding does not fix sudo on a host with a resolv.conf search list, which is the environment it exists for (pkg/dns/synthetic.go:78).
One cross-cutting note: design.md:701 and design-l7.md:286-289 now contradict each other on whether startup pre-population mints forward-resolution evidence. Production follows design-l7.md.
Review on #131, two blocking. Startup pre-population resolved every tracked hostname through the stub and minted forward-resolution evidence for each — including _gateway once a rule named it — so ApplyRulesToTrackedHostnames then L7-scoped the gateway address regardless of what the relay withheld. Phase-1 recording is now recordSystemCacheAnswer (cmd/prepopulate.go): it asks dns.Server.LocalAlias and maps such names for attribution without evidence. Pinned on the cmd path. design-l7.md carries the exception; design.md agrees with it. The machine's own name got no search-expansion handling, so on a host with a search list a c-ares client asked <hostname>.<search> first, was REFUSED, and stopped. Its expanded form is now NXDOMAIN like _gateway's. (glibc falls through on REFUSED — getent and sudo already worked here with "search lan" — so the case is c-ares, and it now resolves live.) Also from review: _localdnsstub and _localdnsproxy are relay-only — loopback listeners the relay itself depends on, never tracked or written; the machine name is read per lookup, so a rename mid-run is honoured; the guessed mDNS <label>.local form is gone, since resolved renames it on conflict and would multicast an unsynthesized one; a localhost-family hostname seeds nothing; enforceDNSResponse takes an answerSource (wireAnswer/localAnswer) instead of a positional bool; README states the systemd-resolved requirement and the fall-through. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
jordanjennings
approved these changes
Sep 16, 2026
jordanjennings
left a comment
jordanjennings
left a comment
There was a problem hiding this comment.
One non-blocking item inline on cmd/prepopulate.go.
Separately: the machine FQDN interception — a hostname that is also a real DNS name now resolves to resolved's synthesized address rather than the upstream record — is unchanged and isn't recorded as a residual in design.md. It's inherent to making the machine hostname resolve at all, so I'm noting it rather than asking for a change.
Review on #131 (non-blocking, approved): LocalAlias collapsed both alias classes into one bool, so Phase-1 pre-population still mapped _localdnsstub to 127.0.0.53, and ApplyRulesToTrackedHostnames unions the IP→name map into its replay — a rule naming the stub listener would have written BPF entries for the address the relay and cache peek depend on, the leak the invariant claims to close. ClassifyAlias replaces it: a wire name is mapped and mints evidence, an enforced alias is mapped without evidence, a loopback listener or localhost alias is recorded nowhere. Pinned on the cmd path. design.md records the residual the review noted: a machine hostname that is also a real DNS name resolves to resolved's synthesized address rather than the upstream record — inherent to making it resolve at all, and what the host does natively. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
Review on #131: the same fact was encoded three ways — a 4-bool tuple from lookupSynthetic that could express a state never returned, a 3-way AliasClass projected from it that folded two query-path behaviours together, and answerSource — and Phase-1 recording restated the live path's policy as a switch in cmd. lookupSynthetic now returns (name, aliasClass) with the missing case: notAlias, trackedAlias, untrackedAlias, expandedAlias. serveSynthetic switches on it. The sets are checked in place; nothing is built per query, and the machine hostname is two strings, not a slice. Phase-1 recording is Server.RecordSystemCacheAnswer — the proxy's own policy, called from cmd/start.go — so ClassifyAlias and cmd/prepopulate.go are gone and cmd re-encodes nothing. answerSource stays as the evidence contract for enforceDNSResponse, now beside it in server.go. No behaviour change; tests re-pinned against the class. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
Review on #131 (blocking): machineHostnames reports "" for an absent form — no first label when the hostname is a single label, the common runner case, or no hostname when gethostname fails — and the exact match compared full == label unguarded. The root query "." trims to "", so it matched the absent label and was relayed to the stub as a tracked alias instead of taking the ordinary path. The slice version could not do this; the two-string rewrite needed the same "" guard the expansion path already had. Both exact comparisons now require a non-empty form. Pinned with a single-label hostname and a failed gethostname against ".", alongside the real single-label name still matching. Signed-off-by: Matthew DeVenny <matt@codecargo.com>
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.
Part of #129 — the agent half. The control-plane half (admitting underscore labels in
CargoWallValidationHelpers.BeValidHostname, plus thecargowall_add_allow_ruledescription and a unit test class) is a companion change inapp, left unstaged there for review per that repo's conventions. #129 closes when both land.Problem
#128 made
_gatewayresolve again, but deliberately as resolution only: the relayed answer fed neither hostname tracking nor the firewall. So on a runner whose host agent sits at the default gateway (Blacksmith:BLACKSMITH_AGENT_ADDR=192.168.127.1) the operator sees a block on a bare IP with no name attached, and the only fix is a CIDR that is one runner generation's gvproxy detail.Change
Which synthetic names are enforced — one classifier,
lookupSynthetic→ (name,aliasClass:notAlias/trackedAlias/untrackedAlias/expandedAlias), checked in place against the fixed sets with nothing built per query:_gateway,_outboundand the machine's own hostname (its configured name and, when it carries a domain, its first label — read per lookup, so ahostnamectl set-hostnamemid-run is honoured;/etc/hostsdoes not always pin it, and under the redirectgetent hosts $(hostname)failed andsudowarned on every invocation) get L4 enforcement and attribution like an upstream answer: an allow rule opens the address on its ports, a deny closes it, no rule → the block is attributed to the name instead of a bare IP._localdnsstub,_localdnsproxyand the localhost family are relay-only: loopback listeners the relay itself depends on, and any label under.localhostis accepted, so tracking them would growhostnameIPswithout bound.<name>.<search>) is NXDOMAIN for every one of these, as #126 answer systemd-resolved synthetic names in the DNS proxy #128 did for_gateway— REFUSED ends a c-ares search before the bare name. The mDNS<label>.localform is not relayed: resolved renames it on conflict, and an unsynthesized.localquery is multicast to the LAN.No L7 identity for an alias.
_gatewayis an address alias, never a name a peer presents; binding it would L7-scope the address against a name no flow can show, and an all-ports allow would then drop HTTP to the host agent for carryingHost: 192.168.127.1. Two paths mint forward-resolution evidence and both now withhold it for such names:enforceDNSResponsetakes ananswerSource—wireAnswerfrom the upstream and CNAME pre-resolve callers,localAnswerfrom the relay. The shared function knows nothing of synthetic names.dns.Server.RecordSystemCacheAnswer, socmdrestates no policy: a wire name is mapped and mints evidence, a tracked alias is mapped without evidence, a loopback listener is recorded nowhere — so the tracked-hostname replay can never write the stub's own address. Without this, a_gatewayrule plus--prepopulate-dns-cache(both presets) L7-scoped the gateway regardless of what the relay did.SCOPE IFF BOUND makes the omission sufficient; nothing in the L7 matcher learns about synthetic names. The rule is L4-only, like the CIDR it replaces.
Verification
Lima, enforce + query filtering,
search lan,curl 192.168.5.2:8080(the VM's gateway, nothing listening):_gatewayallow rulegetent hosts _gateway192.168.5.2192.168.5.2Connection blocked … dst=_gateway dst_ip=192.168.5.2 dst_port=8080DNS resolution: added to firewall hostname=_gateway … action=allow, thenConnection allowed … dst=_gatewaysudo<hostname>.lanon the wireUnit (
pkg/dns): relay tracked-not-cached with no firewall call absent a rule; allow →AddIP(192.168.5.2, allow, tcp/1041), deny under default allow →AddIP(…, deny);AllowDoesNotL7Scope(no evidence, registrar untouched, attribution intact) with the wire-path contrast (myservice.corp.lanmints evidence,all-portsscope);StubListenersUntracked;MachineHostnameRelayedAndTrackedincl. expanded-form NXDOMAIN and a mid-run rename;TestMachineNamesincl. localhost screening;TestRecordSystemCacheAnsweron the pre-population path (all three classes);TestLookupSynthetic(25 cases). The Lima checks are L4-only (--tls-snioff; nothing listens at the gateway on an HTTP port): the L7 seam is pinned by the unit tests plusTestRegisterL7ScopesOnlyBoundIPs. Full suite green; vet, staticcheck, gofumpt, goimports-reviser clean.Docs
README (
allowed-hosts): underscore labels are valid;_gatewaynames the runner's default gateway; the rule is L4-only and trusts the routing table, scope it to repo/workflow; it needs systemd-resolved — without a stub the name does not resolve and the rule opens nothing. design.md's synthetic-names bullet and design-l7.md's pre-population paragraph agree on evidence.Not done here
Auto-allowing the gateway — reachability stays an explicit rule. Recommending
_gatewayat org level. Residual recorded in design.md: a machine hostname that is also a real DNS name resolves to resolved's synthesized address rather than the upstream record — inherent to making the machine hostname resolve, and what the host does natively.🤖 Generated with Claude Code