fix(network): keep earlier dns answers bound to a domain - #1747
Open
yapadesouci wants to merge 6 commits into
Open
yapadesouci wants to merge 6 commits into
yapadesouci wants to merge 6 commits into
Conversation
toksdotdev
force-pushed
the
fix-dns-pin-set-merge
branch
from
October 3, 2026 00:47
f4ac0c5 to
99d2dac
Compare
yapadesouci
force-pushed
the
fix-dns-pin-set-merge
branch
from
October 3, 2026 11:44
6fc486a to
59ef6da
Compare
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
force-pushed
the
fix-dns-pin-set-merge
branch
from
October 3, 2026 11:54
5930f55 to
16ababd
Compare
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
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.
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.TtlReverseIndex::extend: adds members to a key and keeps the ones already bound. A member bound again keeps the later of its two expiries.insertkeeps its replace semantics, andremovestill drops every member of the key.SharedState::cache_resolved_hostnameusesextend.clear_resolved_hostnameis unchanged: an answer with no addresses for a family still clears that family.docs/networking/dns.mdxgains one sentence saying how long a recorded address stays valid.docs/security/network.mdxalready describes for the DNS pin set. Forged SNI and hard-coded IPs are still refused.Test Plan
cargo fmt --all -- --checkcargo clippy -p microsandbox-utils -p microsandbox-network --all-targets -- -D warningscargo test -p microsandbox-utils -p microsandbox-network: 686 passed, including 5 newttl_reverse_indextests forextendand a newresolved_hostnames_keep_earlier_answerstest innetstack/shared.rs.Real VM on macOS arm64.
msbwas built from this branch (cargo build --release -p microsandbox-cli), codesigned withmsb-entitlements.plist, and run with the v0.7.6libkrunfw.5.dylib. Each run made three rounds of 30 parallelcurldownloads fromfonts.gstatic.comwith--no-net --net-rule allow@fonts.gstatic.com:tcp:443 --dns-nameserver 1.1.1.1onbuildpack-deps:bookworm-curl:--net-strict=false--tls-interceptIn 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.The PR appears safe to merge; no outstanding findings remain.
Reviews (9) · Last reviewed commit: "docs(network): the 64-address limit spar..."