Skip to content

Protect private DNS ownership and make readiness trustworthy #261

Open

Description

@ecv

Summary

Resolve the ownership, recovery, and validation findings from the initial Private DNS review.

Why it matters

A customer must not lose a name or address to another network, and a Ready status must mean the service can actually answer safely.

Acceptance criteria

  • Releasing or changing a DNS address does not leak its allocation, and the same address cannot be assigned to different customers.
  • A newly created network context cannot take over a name owned by an established context.
  • DNS recovers after a restart and does not report Ready while it is intentionally refusing queries.
  • Isolation tests detect a broken lookup path instead of passing without proving that names are isolated.
  • Public or reserved domain suffixes cannot be claimed inappropriately.
  • Each review finding is verified as fixed or recorded as an explicitly accepted exception.

Related work

Parent: Restrict private DNS access to approved networks.

Original report and engineering evidence are preserved in the discussion.

Activity

  1. added
    tech-debtInternal quality/maintenance: unwind shortcuts, config drift, overdue upgrades, fork/dep hygiene
    on Oct 22, 2026
  2. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    Detail by source PR:

    #233, resolver contexts and access

    • ClaimDestination checks (region, address) only against other destination claims. Generated regional addresses live under dns-address-claim-*, so one context's node address can equal another's regional address. It also accepts loopback, link-local and multicast addresses.
    • platform/allocator.go never releases destination claims, so each address edit leaks a ConfigMap and blocks that address for every project for good.
    • A duplicate consumerID is settled by name order, so a later context with an earlier-sorting name takes ownership, and confirmAccessWithdrawal tombstones the incumbent's access. Rank by creationTimestamp, then UID.
    • DNSResolverBinding in the platform namespace is not watched, so serving acknowledgements are seen only on the 15s requeue.

    #237, serving recovery

    • In restoreActive, an ApplyForce error returns without marking activation needed. If the daemons were down at restart, nothing retries until the agent restarts again.
    • probeLocalDaemons accepts any DNS response, REFUSED and SERVFAIL included. failClosed writes no marker, so after a fail-closed and a restart the node can report Ready while gated. Query a known name from the active snapshot.
    • Every restart test sets UnsafeAllowNoDNSVerify: true, so the probe path is never exercised.
    • canRetainActive drops all serving on any rejected record removal or version bump. The body says invalid updates retain accepted state. Decide which, and say so.

    #239, qualification tests

    • The public-recursion check queries live cloudflare.com with DNSSEC, so CI depends on internet egress.
    • The unbound-destination case passes on any transport error. Assert REFUSED or SERVFAIL was received.
    • Both NXDOMAIN isolation checks can pass on the first sample before the owning tenant resolves.

    #243, domain allocation

    • domainSuffix is checked only for DNS-1123 shape, so com or cluster.local becomes a private zone that shadows real names. Add an allowlist such as .internal.
    • The new required-when-enabled rule blocks every update, finalizer removal included, to enabled contexts created without domainSuffix.
    • Status.ManagedNamespace is zeroed on every reconcile and refilled only when ready, so it flaps.
  3. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    Three more follow-ups from #230, which approved without them:

    • Nothing rejects a private zone that shares an apex with a public zone in the same project. The controller skips the Domain claim for private zones, so the outcome is undefined.
    • internal/agent/reader.go and the CLI zone import still list private zones and their records.
    • Applying the CRD before the new controller rolls leaves a window where an older controller sends a new private zone to PowerDNS. The release note should say not to create private zones until the controller has rolled.
  4. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    More follow-ups, from #232 at 29eea5f (approved once unsupported events stopped being terminated):

    • MaxAckPending is 64 and Retry uses NakWithDelay, so redelivery can put an activation ahead of its chunks. Use MaxAckPending=1 for publication consumers, or test that receivers buffer by dependency.
    • An unsupported event is redelivered every minute forever, holding an ack-pending slot. Cap it, or park it and report the version in the member ack.
    • SnapshotStore.Put compares whole envelopes, so a republish with a fresh PublishedAt or EventID fails. Compare the payload hash.
    • Nothing checks an envelope's region, shard and kind against its subject, or binds an ack's MemberID to its subject.
    • ZoneAttachment.Pending is unusable, because ServingSnapshot.Validate requires a nonzero epoch and revision on every attachment.
    • The acknowledgement lists have no listType=map or MaxItems, and a zoneApices change needs no generation or revision bump.
    • .internal-dns-e2e/ is not in .gitignore. Rotate the NATS passwords from that directory if they were used anywhere real.
  5. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    From #236 at e63c21a (approved once UDP PROXY was removed):

    • All contexts share one public forwarder with no concurrency cap and no per-tenant rate or TCP connection limit, so one VPC can starve the rest.
    • Replace rebuilds every context's cache on any change.
    • A config that fails to decode on reload keeps serving the old zones. Decide whether that is accepted.
    • The service now answers only TCP. Say so wherever resolvers are configured.
  6. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    From #234 at 6111210 and #235 at f2e0a60 (both approved once their blockers were fixed):

  7. ecv commented on Oct 22, 2026

    @ecv
    ContributorAuthor

    Status after #233 at ae2a3d6 and #237 at 0f08c61.

    Now fixed: non-routable destinations are rejected, destinations inside the regional prefix or with an existing regional claim are refused, duplicate consumerID ownership goes to the oldest context, a failed restore marks activation needed, and a fail-closed persists its marker across restarts.

    Still open:

    • Destination claims are never released, so each address edit still burns that address for every project.
    • DNSResolverBinding is still not watched.
    • A zoned address such as fd54::1%eth0 passes IsGlobalUnicast and misses Prefix.Contains, skipping both checks. Strip or reject the zone in ClaimDestination.
    • reserve does not skip a candidate that already holds a destination claim.
    • probeLocalDaemons still accepts REFUSED and SERVFAIL, and the restart tests still never run the probe.
    • With the gate now persisted, a rejected removal or version bump keeps a node closed across restarts until a valid snapshot arrives. That makes the retain-on-reject decision more urgent.
    • A failing restore now retries at 1 Hz with no backoff, and each pass restarts the owned daemons.
  8. added theissue type on Oct 29, 2026
  9. scotwells commented on Oct 29, 2026

    @scotwells
    Contributor

    The description now explains the problem and expected outcome in plain language. The original report is preserved below for the observations, diagnostics, and implementation details behind this work. It describes the situation when the issue was reported; later comments and the issue state record subsequent progress.

    Original report and engineering evidence

    The internal DNS stack has follow-ups that did not block its review but will bite in production. They come from feat: Manage private DNS contexts and resolver access, feat: Preserve private DNS isolation through serving failures, test: Validate internal DNS on the shared Kubernetes environment and Allocate private DNS domains requested by consumer services.

    1. Address claims are never released, and two claim types can hand one address to two tenants.
    2. A newly created resolver context can take a name from an established one.
    3. A serving node can get stuck not serving after a restart, and can report Ready while gated.
    4. Several isolation tests pass on a broken path.
    5. A context can claim a public or reserved domain suffix.

    Done when each item is fixed or recorded as accepted. The detail, with file references, is in the first comment.

    Related to feat: Manage private DNS contexts and resolver access, feat: Preserve private DNS isolation through serving failures, test: Validate internal DNS on the shared Kubernetes environment, Allocate private DNS domains requested by consumer services

  10. changed the title [-]Internal DNS can leak addresses, hand over names and report Ready falsely[/-] [+]Protect private DNS ownership and make readiness trustworthy[/+] on Oct 29, 2026
  11. scotwells commented on Oct 29, 2026

    @scotwells
    Contributor

    Fresh staging checks on 29 October verified two isolated Kata networks resolve the same private name to independent addresses through both advertised resolvers over UDP and TCP. The east matrix passed all 24 queries, covering both its regional-forwarding and local-answer profiles; four initial Kubernetes proxy errors happened before DNS execution and passed on bounded retry.

    A second zone became available only after explicit association (first observed answer 7.30 s after creation); removing it stopped answers after 4.14 s. Recreated zones and networks received new identities, and old associations stayed invalid until explicitly recreated. Restoring owner access did not restore the revoked network. Stale zone replacement was rejected by UID precondition. These are observed timings, not agreed service-target passes.

    An approved one-time secondary-resolver replacement also preserved denial: the access monitor saw 87 NXDOMAIN replies and 3 transport failures for the revoked network, with zero unauthorized answers, while its separate authorized-network control returned 90/90 correct replies. Post-recovery checks passed both protocols and addresses. Restart measurements record the interruption separately.

    Public authoritative checks returned REFUSED/no private answers on both staging IPv4 endpoints over UDP/TCP, with successful public SOA controls. Still incomplete: delayed broker replay, authorization/ownership attack cases, regional withdrawal, dependency outage/lease expiry, and a deliberate software isolation-failure test. Predicate sensitivity against deliberately granted access was checked, but is narrower than that fault test. No issue acceptance criteria are being closed from these samples.

  12. scotwells commented on Oct 29, 2026

    @scotwells
    Contributor

    Final cleanup verification adds a remaining access-state gap: the source/edge test resources are gone and all test record names are absent from inspected active configurations, but one deleted network's forwarding route remained on central node-b over three minutes later. No stale private answer was demonstrated; full active-state cleanup is not confirmed. The new deleted-network routing issue, attached to the access epic, records the evidence, limits and acceptance checks. No additional restart or shared checkpoint deletion was performed.

  13. self-assigned this
    on Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

tech-debtInternal quality/maintenance: unwind shortcuts, config drift, overdue upgrades, fork/dep hygiene

Type

Fields

Priority

None yet

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions