Repository navigation
feat: allowed client networks on CoreDNS deployments and measured certificates for external engines - #234
Merged
Conversation
…NS deployment lattice-sdk feature/20261025_s5bc-dns-deployments adds DNSDeployment.AllowedClients and CertMonitorIDs (design 32 S5b and S5c), both additive. go.mod and sdk.ref name the same commit. Re-pin after the SDK branch merges.
Design 32 S5b. A CoreDNS deployment takes allowed_clients, at most 64 CIDRs, IPv4 or IPv6, with no host bits set. They render as the acl plugin in every server block: one allow net per network, then a bare block, because the plugin's default action is allow and a query no rule matches would otherwise recurse. 0.0.0.0/0 and ::/0 are refused, since they reopen the resolver, and so is an IPv4 network written as IPv6. S0 refused a public deployment with a forward zone outright. It now saves and plans when it names its clients, and the refusal without them lands on allowed_clients, the field that answers it. A stored one keeps loading with its flag. The approval gate judges the plan text, so it now reads each forwarding server block and calls it open only when no acl ending in a bare block guards it. The plan gains an "Allowed clients" section saying the ACL decides who gets answers, not who reaches the port; the NetGuard listener check is unchanged. An external engine refuses the field, since nothing is rendered for it. Lattice pins no CoreDNS binary in code: the server's LATTICE_COREDNS_BINARY_* names one, or the node's PATH supplies it. acl is in upstream's default plugin.cfg (checked at v1.6.9, v1.7.0 and v1.12.4, the README example), and coredns -plugins lists it as "acl" (older builds "dns.acl"). Since the configured binary cannot be seen from here, the apply script asks it before writing anything when the Corefile uses acl, and stops with the reason if it lacks the plugin. Tested: go test ./internal/selfdns/; the server DNS tests by -run.
Design 32 S5c. An external engine's record carries a certificate expiry the operator typed, which nothing checks. POST /api/dns/measure-cert turns the DoT and DoH listeners the operator names into tls checks at hostname:port and links them to the deployment, so its view shows the measured expiry and says so, and falls back to the typed date while nothing has been measured. Creation lives on the server so the checks and the link land in one store write (LinkDNSCertMonitors), and the link is the deployment's cert_monitor_ids, which a save never writes: UpsertDNSDeployment keeps the stored list, so an operator's save that read the record before a measure cannot drop it. An authored tls check already on the endpoint is linked rather than duplicated, which also makes a second measure, or one after a failed write, create nothing twice. The checks are ordinary operator checks with no source: the control plane evaluates tls checks itself, so the spec's "source the operator picks" has nothing to pick here. The record lists listeners as tcp or udp and a port, not which speak TLS (production's DoH listens on 8443), so the request names the ports; each must be a recorded tcp listener. The caller needs dns:admin on the node and the tls create rule, monitor:admin on a token with no node allowlist. The view gains cert_checks, the linked checks the caller may read with their newest measured not-after, and cert_expiry: the soonest expiry an enabled linked check measured, else the typed one, with its source. A deleted check drops out of the link on the next measure and is skipped on read; deleting the deployment leaves its checks. Tested: store LinkDNSCertMonitors tests; server measure tests by -run.
…n unknown Corefile shape open Review of S5b. 0.0.0.0/1 with 128.0.0.0/1, or ::/1 with 8000::/1, passed the /0 refusal and still answered everyone. NormalizeAllowedClients now merges each family's networks and refuses the list when they cover all of it. The plan-level open-resolver judgement read only the shape this server renders and failed open on anything else: CRLF line ends never closed a block, so a guarded block's verdict carried into the next. It now drops carriage returns and counts any column-zero line inside a block as open. The apply's acl check names the plugin as dns.acl and says the running Corefile was not replaced.
A tls check already sent its target's host as SNI but read whatever leaf came back. A certificate for another name is not that endpoint's certificate, and its expiry is not the one being watched: the run now fails with the reason and records no expiry, so Expiring and a DNS deployment's measured certificate keep the last reading of the right certificate. The chain is still not verified, so an expired or incompletely chained certificate still reads. This applies to every tls check on a host name, not only the ones a DNS deployment's measure creates. A check on an IP address is not name-checked, since most certificates carry no address.
… record that was read Lead and review conditions on S5c. The measure refuses a record with no hostname, or with an address in its place, in a sentence that names hostname; an external engine's save now refuses an address as its hostname. The checks therefore always dial the name, which SNI asks for and the probe now holds the certificate to. The link is written only if the deployment is still the record the handler read and authorized against (LinkDNSCertMonitors takes its UpdatedAt), so a save in between that moves it to another node or hostname answers 409 and links nothing. A linked check counts only while it dials an endpoint the record lists, its hostname on a recorded tcp port. After a hostname or listener edit, or an edit of the check's target, it drops out of the sheet and stays an ordinary check under Monitoring. Tests: a token holding only dns:admin or only monitor:admin is refused and nothing is created or linked; the hostname refusals; a dropped listener drops its check.
…nother host The name check from 9aa55b1 recorded no expiry for a certificate that did not cover the target's host, which broke the probe's contract: it skips verification so that it can always read the expiry, the cases where a verifying handshake would refuse being the ones the operator most needs the number for. A completed handshake now always records cert_not_after again. On a host-name target a leaf that does not cover the host fails the run with a reason naming the host and the certificate's DNS names (at most five, then a count); an expired certificate and the threshold work as before, and when both fail the reasons are joined. Address targets are unchanged. The doc comments say what the probe now asserts. Tested: a mismatched certificate fails with the names and still records its expiry; a matching wildcard passes and one two labels deep does not; the names list is bounded.
…25_s5bc-dns-deployments
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
Design 32 S5b and S5c, server half. Commits: the SDK pin, S5b, S5c, then three follow-ups for the lead's conditions and an independent review (refuse a family-covering allowed list and judge unknown Corefile shapes open; hold a tls check's certificate to its host name; hostname-only measure, link only the record read, stale links ignored).
S5b: allowed client networks
allowed_clientson a CoreDNS deployment: at most 64 CIDRs, IPv4 or IPv6, no host bits;0.0.0.0/0,::/0, IPv4-in-IPv6 networks, and any list whose networks together cover a whole family (such as0.0.0.0/1with128.0.0.0/1) refused. One validator (selfdns.NormalizeAllowedClients) serves the save path and the renderer, and a refusal names the entry (allowed_clients[i]) so the editor focuses it (R6).aclplugin in every server block, ahead of the zone plugin: oneallow netper network, then a bareblock(REFUSED), because the plugin's default action is allow.allowed_clientsis not empty, on save and on plan; without it the refusal now lands onallowed_clients. A stored one keeps loading with its flag. The approval gate (PlanIsOpenResolver) judges the plan text per server block: a forwarding block is open unless an acl whose last rule is a bareblockguards it, and any shape the renderer does not write (CRLF, a block that never closes) counts as open.allowed_clients.CoreDNS
aclin the pinned binary. Lattice pins no CoreDNS binary in code: the server'sLATTICE_COREDNS_BINARY_VERSION/URL/SHA256name one (the README example is 1.12.4), and without them the node'scorednsin PATH is used. I could not see the production env file from here. Upstream's defaultplugin.cfgincludesacl:aclat v1.6.9, v1.7.0 and v1.12.4 (checked against github.com/coredns/coredns), andcoredns -pluginsprints it asacl(older buildsdns.acl, per coredns/caddyDescribePlugins). Because the binary actually in use cannot be checked from the control plane, the apply script the server renders (no node-agent change) asks it, before writing anything, when the Corefile usesacl:coredns -pluginsmust listdns.acl(or bareacl), and otherwise the apply stops with "this coredns binary lacks the acl plugin (dns.acl) ... the running Corefile was not replaced".For the lead before any deploy: the production binary pin is
LATTICE_COREDNS_BINARY_VERSION,LATTICE_COREDNS_BINARY_URLandLATTICE_COREDNS_BINARY_SHA256in the server's.envon hkg, which this lane cannot see. Verify that it names an upstream CoreDNS build (or that the nodes' PATHcorednsis one) before the first plan withallowed_clients.S5c: measured certificates for external engines
POST /api/dns/measure-cert {id, ports, threshold_days?}creates one server-evaluated tls check per named DoT/DoH listener athostname:portand links them to the deployment in one store write (LinkDNSCertMonitors). That is why creation lives on the server rather than in the console: the checks and the link land together or not at all.DNSDeployment.cert_monitor_ids, andUpsertDNSDeploymentalways keeps the stored list, so an operator save that read the record before a measure cannot drop it, and a request cannot link checks by carrying ids.dns.roobli.org:8443watch gets linked in one click.cert_checks(linked checks the caller may read, with their newest measured not-after) andcert_expiry(the soonest expiry an enabled linked check measured, else the typedcert_not_after, withsource).dns:adminon the deployment's node (as a save) and the tls create rule (monitor:adminon a token with no node allowlist). A token with only one of them is refused and nothing is created or linked (tested). Auditsmonitor.createper new check anddns.cert.measure.hostname, and an external engine's save now refuses an address as its hostname. SNI already carried the host; the probe now also holds the leaf to that name (see "Behaviour change" below).LinkDNSCertMonitorstakes itsUpdatedAt; otherwise 409, nothing written).Behaviour change
Every tls check whose target is a host name, not only the ones a measure creates, now checks that the leaf certificate covers that host (
x509.Certificate.VerifyHostname, wildcards included). An existing check whose certificate does not cover its host will start failing with "the certificate does not cover HOST (it names A, B, ...)" (at most five names, then a count). The expiry is still recorded on every completed handshake, as before, so Expiring and the measured expiry keep reading it; the chain is still not verified, so an expired or incompletely chained certificate still reads; the threshold works as before, and when both fail the reasons are joined. Checks on an IP address are unchanged. The lead will check the production tls checks before any deploy.Decisions a reviewer would question
server_monitor_tls.go). Measuring from a vantage point is what an S5a dns check gives later.DNSListenerrecords tcp/udp and a port, not which speak TLS (production DoH is on 8443, and 2053 is on both udp and tcp), so the request lists the ports and each must be a recorded tcp listener. The console suggests tcp listeners not on 53 with no udp twin. I did not add a TLS marker toDNSListener.Edge cases handled and left out
Handled: a deleted check drops out of the link on the next measure and is skipped on read; the deployment deleted, switched engine or saved mid-measure (404/409, nothing written); a linked check deleted mid-measure (409, nothing written); a check whose endpoint the record no longer lists; duplicate ports; a node-confined or one-scope token (403). Left out: deleting a deployment leaves its checks (they are ordinary operator checks, and the console says so); a session that can read the deployment but not the checks sees the typed date labelled as typed; a stale linked check is ignored, not unlinked or deleted.
Test plan
gofmt -l .cleango test ./internal/selfdns/(new: acl render before the zones, refusals, plan text, plan-level open-resolver judgement, apply-script acl check; confirmed failing against the old renderer)LinkDNSCertMonitorsone-write link, save keeps the link, refusals write nothingallowed_clientsand is refused without (fieldallowed_clients),0.0.0.0/0refused onallowed_clients[1], external refused, plan carries the acl and approves; measure creates one tls check per listener, links the hand-made one, is idempotent, sheet prefers the measured expiry, an edit keeps the link, refusalsgo test -timeout 40m ./...locally, non-race, on f10bd54 and again on 9c5b5a4 (internal/server 232 s, internal/store 60 s); the last commit, which only changes the tls probe's verdict, by focused-run(TLS monitor, measure, Expiring)