Repository navigation
[Bubblewrap] Enforce proxy-only egress for private networking #931
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,8 +98,8 @@ pub(crate) enum ResolvedNetworkMode { | |
| LegacyProxy, | ||
| /// The host network namespace with per-destination iptables filtering. | ||
| FirewallFiltered, | ||
| /// Cooperative proxy routing inside a slirp-backed private namespace, with | ||
| /// egress closed to everything but the proxy. | ||
|
Collaborator
There was a problem hiding this comment. issue: The existing config warning now contradicts this schema 0.8+ guarantee.
Could we scope that warning to the legacy proxy path (schema 0.6/0.7), or otherwise suppress it for schema 0.8+
Contributor
Author
There was a problem hiding this comment. Good catch, and agreed the warning is wrong under 0.8. Tracked in #970 (Priority3) rather than fixed here. |
||
| ProxyOnly, | ||
| } | ||
|
|
||
|
|
@@ -152,9 +152,13 @@ impl ResolvedNetworkMode { | |
| matches!(self, Self::ProxyOnly) | ||
| } | ||
|
|
||
| /// Whether the mode uses the host-side firewall manager. | ||
| /// | ||
| /// False for `ProxyOnly`, which is *not* iptables-free: it programs rules | ||
| /// directly into the sandbox's own network namespace from the supervisor | ||
| /// (see `proxy_network`) rather than going through the host manager. | ||
| #[cfg(any(target_os = "linux", test))] | ||
| pub(crate) fn requires_host_firewall_manager(self) -> bool { | ||
| matches!(self, Self::FirewallFiltered) | ||
| } | ||
| } | ||
|
|
@@ -566,8 +570,10 @@ mod tests { | |
| assert!(ResolvedNetworkMode::ProxyOnly.uses_external_userns()); | ||
| assert!(!ResolvedNetworkMode::Isolated.uses_external_userns()); | ||
| assert!(!ResolvedNetworkMode::LegacyProxy.uses_private_netns()); | ||
| assert!(ResolvedNetworkMode::FirewallFiltered.requires_host_firewall_manager()); | ||
| assert!(!ResolvedNetworkMode::Shared.requires_host_firewall_manager()); | ||
| // ProxyOnly enforces via in-netns rules, not the host manager. | ||
| assert!(!ResolvedNetworkMode::ProxyOnly.requires_host_firewall_manager()); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: nice! I do something similar for Windows in AppInfoSvc. That said, I'd be interested in when we clean up things. In Windows we clean up the Windows Filtering Platform rules I add once all processes in the Job object has gone away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleanup here is structural rather than bookkeeping: the rules live inside the sandbox's own network namespace, not the host's tables. When the last process in that namespace exits, the kernel tears the namespace down and every rule goes with it — so there is nothing to reconcile, and no way for a rule to outlive the sandbox or affect anything else on the host.
Drop for ProxyNetworkNamespacestill explicitly reaps the supervisor, slirp4netns and the temp state dir.The closer analogue to your WFP case is the legacy
firewallmode, which does write host-global rules and needs explicit removal — one of the reasons 0.8 moved the rules inside the namespace.