fix(network-vmnet): install explicit subnet route, not just interface-scoped default#1882
Open
ryan106 wants to merge 1 commit into
Open
fix(network-vmnet): install explicit subnet route, not just interface-scoped default#1882ryan106 wants to merge 1 commit into
ryan106 wants to merge 1 commit into
Conversation
…-scoped default vmnet_network_create() only results in an interface-scoped default route for the container subnet, never an explicit non-scoped route. Routing to containers is therefore entirely dependent on which global default route currently wins on the host. Any other tool that installs its own default route (a VPN client, a Tailscale exit node, etc.) can shadow it and silently break all container connectivity, even though this subnet's own route should normally win under standard longest-prefix-match rules. Install an explicit route ourselves via route(8), matching the existing Process-based pattern this codebase already uses for pfctl in PacketFilter.swift. Best-effort: failures are logged, not thrown, since this is a resilience improvement rather than a hard requirement for the network to otherwise start successfully. It also doesn't fully eliminate exposure to tools that actively remove routes they don't own (see tailscale/tailscale#18653) — only to the simpler and more common case of a competing default route being added without deleting others. Fixes apple#1881.
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.
Type of Change
Motivation and Context
vmnet_network_create()only results in an interface-scoped default route for the container subnet (e.g.bridge100), never an explicit non-scoped route for the subnet itself (e.g.192.168.64.0/24). Because of that, routing to containers is entirely dependent on which global default route currently wins on the host. Any other tool that installs or changes the system's default route (a VPN client, a Tailscale exit node, etc.) can shadow it and silently break all container connectivity — even though a specific/24route for this subnet should normally win over any/0default route under standard longest-prefix-match rules, since no such specific route exists to begin with.Repro (full details in #1881):
nc -zv <container-ip> <port>succeeds).route -n get <container-ip>now resolves via the wrong interface; connectivity breaks.container system stop && container system start, does not restore connectivity — there was never a specific subnet route to fall back to, only the interface-scoped default.route add -net <subnet> -netmask <mask> -interface <gateway-ip>) does restore it immediately.This PR installs that explicit route automatically when a network starts, using
route(8)viaProcess— the same pattern this codebase already uses forpfctlinPacketFilter.swift.Two things this does not claim to fix, so I want to be upfront about scope:
apple/container'sbridge100interface by name. This change closes the more common case (a competing default route added without deleting others) and givescontainera correct baseline instead of none, but a tool like Tailscale can still remove it afterward.Testing
Tested locally
Added/updated tests
Added/updated docs
swift build --target ContainerNetworkVmnetServercompiles cleanly.I manually validated the underlying
route(8)invocation this PR automates, against the exact broken scenario from [Bug]: Container subnet routing breaks when a competing default route appears (e.g. VPN/exit-node) and doesn't self-heal #1881: with a container running and connectivity broken (exit node enabled, or freshly aftercontainer systemrestart with no subnet route present), runningimmediately restores
route -n get <container-ip>to resolve via the correct bridge interface, and connectivity to the container succeeds.I was not able to do a full signed end-to-end run of the patched
container-network-vmnetplugin itself — I don't have a code-signing/entitlements setup for local testing of this daemon, the same constraint noted in Add DNS forwarding groundwork #1813's testing notes. I'd appreciate it if a maintainer with that setup could confirm the automated call site behaves the same as the manualroute(8)invocation I validated.Related