-
Notifications
You must be signed in to change notification settings - Fork 62
[LXC] Address network policy gaps - model 2 (deny-all-except-proxy) #632
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
5f5ae13
64c072e
6425119
775dc87
9648284
3fd32ed
f22f839
80fb608
4c71e2e
7389263
81362b6
8df9dff
84ac24c
29cf20e
ed6ee5b
a273708
7c4e03b
c5484b1
f276fb2
815758a
fff3c93
c917434
a7a6194
6e7591c
1ed2ae6
63578db
d563cbb
c3af5f3
6756a3b
6c52d93
17ea610
96042f4
10f0599
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -192,13 +192,14 @@ impl LxcScriptRunner { | |
| let _ = writeln!(logger, "Container already running."); | ||
| } | ||
|
|
||
| // Wait for network only when the config uses network features (firewall rules | ||
| // or allowed/blocked hosts). | ||
| // Wait for network only when the config uses network features | ||
| // (firewall rules, allowed/blocked hosts, or proxy enforcement). | ||
| let needs_network = matches!( | ||
| request.policy.network_enforcement_mode, | ||
| NetworkEnforcementMode::Firewall | NetworkEnforcementMode::Both | ||
| ) || !request.policy.allowed_hosts.is_empty() | ||
| || !request.policy.blocked_hosts.is_empty(); | ||
| || !request.policy.blocked_hosts.is_empty() | ||
| || request.policy.network_proxy.is_enabled(); | ||
|
|
||
| if needs_network { | ||
| Self::wait_for_network(&container_name, Duration::from_secs(10), logger); | ||
|
|
@@ -207,6 +208,27 @@ impl LxcScriptRunner { | |
| // Configure network rules | ||
| let mut fw_manager = NetworkIptablesManager::new(&container_name); | ||
|
|
||
| // Pin a hostname proxy to the address the host resolved, once, before | ||
| // anything consumes it. The firewall ACCEPT and the HTTP(S)_PROXY handed | ||
| // to the container must name the same endpoint: if the container | ||
| // re-resolved the hostname itself it could pick a different address | ||
| // under round-robin or split-horizon DNS and be dropped by its own | ||
| // policy. Pinning also means the container never needs a resolver, so | ||
| // DNS stays closed under deny-all-except-proxy. | ||
| let mut effective_policy = request.policy.clone(); | ||
| match NetworkIptablesManager::pin_proxy_to_resolved_ip( | ||
| &effective_policy.network_proxy, | ||
| logger, | ||
| ) { | ||
| Ok(pinned) => effective_policy.network_proxy = pinned, | ||
| Err(e) => { | ||
| if self.destroy_on_exit || container_created { | ||
| let _ = container.destroy(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with bot. Shall we fix this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| } | ||
| return ScriptResponse::error(&format!("Network policy error: {}", e)); | ||
| } | ||
| } | ||
|
|
||
| // Try to discover the container's veth interface for scoped rules | ||
| if let Some(veth) = NetworkIptablesManager::discover_veth_interface(&container_name) { | ||
| let _ = writeln!(logger, "Discovered veth interface: {}", veth); | ||
|
|
@@ -218,7 +240,7 @@ impl LxcScriptRunner { | |
| } | ||
| } | ||
|
|
||
| match fw_manager.apply_firewall_rules(&request.policy, logger) { | ||
| match fw_manager.apply_firewall_rules(&effective_policy, logger) { | ||
| Ok(true) => {} | ||
| Ok(false) => { | ||
| if self.destroy_on_exit || container_created { | ||
|
|
@@ -242,10 +264,14 @@ impl LxcScriptRunner { | |
| Some(Duration::from_millis(u64::from(request.script_timeout))) | ||
| }; | ||
| let _ = writeln!(logger, "Executing script inside container..."); | ||
| let mut exec_env = request.env.clone(); | ||
| let force_clear_env = | ||
| wxc_common::proxy_env::apply_proxy_env(&mut exec_env, &effective_policy.network_proxy); | ||
| let result = container.attach_run( | ||
| &request.script_code, | ||
| &request.working_directory, | ||
| &request.env, | ||
| &exec_env, | ||
| force_clear_env, | ||
| timeout, | ||
| ); | ||
|
|
||
|
|
||
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.
Shall we address this?
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.
Confirmed, and it is not fixed in this PR.
container.start()is atlxc_runner.rs:330, the optional address wait at:358, veth discovery at:361, andapply_firewall_rulesonly at:408, so anything already inside the container has unfiltered egress for that whole span. Moving proxy pinning ahead of start removed host DNS work from the window but did not close it.Closing it properly needs a veth-scoped quarantine chain installed the moment the interface exists and then atomically swapped for the real policy, which is a new enforcement stage with its own rollback and teardown semantics rather than a reordering. I filed #764 for it rather than growing this PR further. The window is established by reading the call ordering, not by observing a packet escape.