Skip to content

fix(network): keep earlier dns answers bound to a domain - #1747

Open
yapadesouci wants to merge 6 commits into
superradcompany:mainfrom
yapadesouci:fix-dns-pin-set-merge
Open

yapadesouci wants to merge 6 commits into
superradcompany:mainfrom
yapadesouci:fix-dns-pin-set-merge

Conversation

@yapadesouci

@yapadesouci yapadesouci commented Oct 2, 2026 •

Copy link
Copy Markdown

TL;DR

Domain allow rules refused most parallel connections to hosts whose DNS answers rotate, because each answer replaced the addresses bound to the name. Answers now accumulate, and each address expires on its own TTL. Fixes #1745.

Description

  • TtlReverseIndex (crates/utils) tracks an expiry per key and member instead of one per key. The versioned expiry events are now per key and member, so a stale timer still cannot remove a newer binding.
  • New TtlReverseIndex::extend: adds members to a key and keeps the ones already bound. A member bound again keeps the later of its two expiries. insert keeps its replace semantics, and remove still drops every member of the key.
  • SharedState::cache_resolved_hostname uses extend. clear_resolved_hostname is unchanged: an answer with no addresses for a family still clears that family.
  • docs/networking/dns.mdx gains one sentence saying how long a recorded address stays valid.
  • Policy impact: every address that stays bound was returned for that name, to this sandbox, within its TTL. That is the contract docs/security/network.mdx already describes for the DNS pin set. Forged SNI and hard-coded IPs are still refused.
  • Compatibility: the change is limited to in-memory state of the network engine. It touches no persisted data, wire format or SDK/runtime boundary.
  • Memory: a name now holds every distinct address it was answered with during one TTL window, instead of one answer's worth. Expired members are evicted as before.

Test Plan

  • cargo fmt --all -- --check

  • cargo clippy -p microsandbox-utils -p microsandbox-network --all-targets -- -D warnings

  • cargo test -p microsandbox-utils -p microsandbox-network: 686 passed, including 5 new ttl_reverse_index tests for extend and a new resolved_hostnames_keep_earlier_answers test in netstack/shared.rs.

  • Real VM on macOS arm64. msb was built from this branch (cargo build --release -p microsandbox-cli), codesigned with msb-entitlements.plist, and run with the v0.7.6 libkrunfw.5.dylib. Each run made three rounds of 30 parallel curl downloads from fonts.gstatic.com with --no-net --net-rule allow@fonts.gstatic.com:tcp:443 --dns-nameserver 1.1.1.1 on buildpack-deps:bookworm-curl:

    --net-strict=false --tls-intercept
    v0.7.6 release 5, 7, 5 of 30 4, 6, 5 of 30
    this branch 30, 30, 30 of 30 30, 30, 30 of 30
  • In the same VM on this branch, an unlisted host (example.com) still fails to resolve. --resolve fonts.gstatic.com:443:8.8.8.8, a forged SNI on an address never resolved for the name, is still refused.

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no outstanding findings remain.

Reviews (9) · Last reviewed commit: "docs(network): the 64-address limit spar..."

Comment thread crates/network/lib/engine/netstack/shared.rs
Comment thread docs/networking/dns.mdx Outdated
@toksdotdev
toksdotdev force-pushed the fix-dns-pin-set-merge branch from f4ac0c5 to 99d2dac Compare October 3, 2026 00:47
Comment thread crates/utils/lib/ttl_reverse_index.rs
Comment thread crates/utils/lib/ttl_reverse_index.rs
Comment thread crates/utils/lib/ttl_reverse_index.rs Outdated
@yapadesouci
yapadesouci force-pushed the fix-dns-pin-set-merge branch from 6fc486a to 59ef6da Compare October 3, 2026 11:44
Comment thread docs/networking/dns.mdx Outdated
Each A/AAAA answer replaced the addresses bound to that name in the
resolved-hostname index. Resolvers that rotate their answers (Google,
CloudFront) give concurrent lookups of one name different addresses,
so only the last answer stayed bound and connections dialed on the
earlier ones were denied by Domain and DomainSuffix allow rules: 30
parallel downloads from fonts.gstatic.com, 5 to 7 succeeded.

TtlReverseIndex now tracks an expiry per key and member, and gains
`extend`, which adds members and keeps the ones already bound until
their own TTL. `insert` keeps its replace semantics. The DNS forwarder
path records answers with `extend`, and an answer with no addresses
for a family still clears that family.

Every address that stays bound was returned for that name within its
TTL, so a domain rule still never matches an address the sandbox did
not resolve.

Fixes superradcompany#1745
…y events

Cap each hostname and family at 64 addresses, dropping the one closest
to expiry, and compact the expiry heap once stale events from refreshed
or cleared bindings outnumber the live ones. Restore the section headers
in ttl_reverse_index.rs and describe the shared answer TTL in the docs.
The per-domain limit dropped the member closest to expiry as each
address was bound, so an answer with more than 64 addresses for one
family evicted its own earlier addresses before their TTL. The index now
binds the whole answer first, then trims the key back to the limit from
the addresses of earlier answers only. A larger answer keeps all its
addresses, and the next answer trims the key again.
@yapadesouci
yapadesouci force-pushed the fix-dns-pin-set-merge branch from 5930f55 to 16ababd Compare October 3, 2026 11:54
@toksdotdev

Copy link
Copy Markdown
Member

hey @yapadesouci, thanks for addressing these. 😃

i already had a local fix in progress. it keeps one expiry entry per binding and uses a sandbox-wide limit, rejecting new answers at capacity instead of dropping addresses whose ttls are still valid. i’ll integrate that with your changes and push it after review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Domain allow rules refuse parallel connections when DNS answers differ: each answer replaces the name's bound addresses

2 participants