mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
windows-sandbox: fail elevated setup when firewall policy is ineffective (#22353)
## Why Elevated Windows sandbox setup currently assumes that the firewall rules it writes will take effect. On managed Windows hosts, local firewall policy changes can be ignored or only partially apply across the active profiles, which means setup can appear to succeed without providing the expected network isolation. ## What changed - Query `INetFwPolicy2::LocalPolicyModifyState` before configuring the elevated sandbox firewall rules. - Fail setup when Windows reports that local firewall policy edits are ineffective or only apply to some current profiles. - Surface that condition with a dedicated `helper_firewall_policy_ineffective` setup error code so support and IT-facing diagnostics can distinguish it from COM access failures. - Add focused coverage for effective policy, group-policy override, and partial-profile coverage cases. ## Testing - `cargo test -p codex-windows-sandbox --bin codex-windows-sandbox-setup`
This commit is contained in:
committed by
GitHub
Unverified
parent
3d2a0b5517
commit
c9edb26755
@@ -2,6 +2,7 @@ use anyhow::Result;
|
||||
use std::fs::File;
|
||||
use std::io::Write;
|
||||
|
||||
use windows::Win32::Foundation::S_OK;
|
||||
use windows::Win32::Foundation::VARIANT_TRUE;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule3;
|
||||
@@ -10,6 +11,8 @@ use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_ACTION_BLOCK;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_ANY;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_TCP;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_UDP;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_MODIFY_STATE;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_MODIFY_STATE_OK;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_PROFILE2_ALL;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_RULE_DIR_OUT;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2;
|
||||
@@ -75,6 +78,7 @@ pub fn ensure_offline_proxy_allowlist(
|
||||
format!("CoCreateInstance NetFwPolicy2 failed: {err:?}"),
|
||||
))
|
||||
})?;
|
||||
ensure_local_policy_rules_take_effect(&policy)?;
|
||||
let rules = policy.Rules().map_err(|err| {
|
||||
anyhow::Error::new(SetupFailure::new(
|
||||
SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
@@ -170,6 +174,7 @@ pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Resul
|
||||
format!("CoCreateInstance NetFwPolicy2 failed: {err:?}"),
|
||||
))
|
||||
})?;
|
||||
ensure_local_policy_rules_take_effect(&policy)?;
|
||||
let rules = policy.Rules().map_err(|err| {
|
||||
anyhow::Error::new(SetupFailure::new(
|
||||
SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
@@ -215,6 +220,52 @@ fn remove_rule_if_present(rules: &INetFwRules, internal_name: &str, log: &mut Fi
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn ensure_local_policy_rules_take_effect(policy: &INetFwPolicy2) -> Result<()> {
|
||||
let mut modify_state = NET_FW_MODIFY_STATE::default();
|
||||
let result = unsafe {
|
||||
(Interface::vtable(policy).LocalPolicyModifyState)(
|
||||
Interface::as_raw(policy),
|
||||
&mut modify_state,
|
||||
)
|
||||
};
|
||||
validate_local_policy_modify_result(result, modify_state)
|
||||
}
|
||||
|
||||
fn validate_local_policy_modify_result(
|
||||
result: windows::core::HRESULT,
|
||||
modify_state: NET_FW_MODIFY_STATE,
|
||||
) -> Result<()> {
|
||||
if result.is_err() {
|
||||
// The COM query itself failed, so Windows never gave us a policy answer.
|
||||
return Err(anyhow::Error::new(SetupFailure::new(
|
||||
SetupErrorCode::HelperFirewallPolicyAccessFailed,
|
||||
format!("INetFwPolicy2::LocalPolicyModifyState failed: {result:?}"),
|
||||
)));
|
||||
}
|
||||
|
||||
if result != S_OK {
|
||||
// S_FALSE means the answer only holds for some active profiles, not all of them.
|
||||
return Err(anyhow::Error::new(SetupFailure::new(
|
||||
SetupErrorCode::HelperFirewallPolicyIneffective,
|
||||
format!(
|
||||
"local firewall policy modifications do not apply to every current profile: LocalPolicyModifyState result={result:?}"
|
||||
),
|
||||
)));
|
||||
}
|
||||
|
||||
if modify_state == NET_FW_MODIFY_STATE_OK {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Windows answered uniformly, and that answer says local rule edits are ineffective.
|
||||
Err(anyhow::Error::new(SetupFailure::new(
|
||||
SetupErrorCode::HelperFirewallPolicyIneffective,
|
||||
format!(
|
||||
"local firewall policy modifications will not take effect: LocalPolicyModifyState={modify_state:?}"
|
||||
),
|
||||
)))
|
||||
}
|
||||
|
||||
fn ensure_block_rule(rules: &INetFwRules, spec: &BlockRuleSpec<'_>, log: &mut File) -> Result<()> {
|
||||
let name = BSTR::from(spec.internal_name);
|
||||
let rule: INetFwRule3 = match unsafe { rules.Item(&name) } {
|
||||
@@ -410,6 +461,10 @@ fn log_line(log: &mut File, msg: &str) -> Result<()> {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
use windows::Win32::Foundation::S_FALSE;
|
||||
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_MODIFY_STATE_GP_OVERRIDE;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
@@ -507,4 +562,37 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_policy_modify_state_accepts_effective_policy() {
|
||||
assert!(validate_local_policy_modify_result(S_OK, NET_FW_MODIFY_STATE_OK).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_policy_modify_state_rejects_ineffective_policy() {
|
||||
let err = validate_local_policy_modify_result(S_OK, NET_FW_MODIFY_STATE_GP_OVERRIDE)
|
||||
.expect_err("group-policy override should fail sandbox firewall setup");
|
||||
let failure = err
|
||||
.downcast_ref::<SetupFailure>()
|
||||
.expect("expected setup failure");
|
||||
|
||||
assert_eq!(
|
||||
failure.code,
|
||||
SetupErrorCode::HelperFirewallPolicyIneffective
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_policy_modify_state_rejects_partial_profile_coverage() {
|
||||
let err = validate_local_policy_modify_result(S_FALSE, NET_FW_MODIFY_STATE_OK)
|
||||
.expect_err("partial profile coverage should fail sandbox firewall setup");
|
||||
let failure = err
|
||||
.downcast_ref::<SetupFailure>()
|
||||
.expect("expected setup failure");
|
||||
|
||||
assert_eq!(
|
||||
failure.code,
|
||||
SetupErrorCode::HelperFirewallPolicyIneffective
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,6 +56,8 @@ pub enum SetupErrorCode {
|
||||
HelperFirewallComInitFailed,
|
||||
/// Helper failed to access firewall policy or rule collections.
|
||||
HelperFirewallPolicyAccessFailed,
|
||||
/// Helper detected that local firewall policy changes will not fully take effect.
|
||||
HelperFirewallPolicyIneffective,
|
||||
/// Helper failed to create, update, or add the firewall rule.
|
||||
HelperFirewallRuleCreateOrAddFailed,
|
||||
/// Helper failed to verify the configured firewall rule scope.
|
||||
@@ -91,6 +93,7 @@ impl SetupErrorCode {
|
||||
Self::HelperCapabilitySidFailed => "helper_capability_sid_failed",
|
||||
Self::HelperFirewallComInitFailed => "helper_firewall_com_init_failed",
|
||||
Self::HelperFirewallPolicyAccessFailed => "helper_firewall_policy_access_failed",
|
||||
Self::HelperFirewallPolicyIneffective => "helper_firewall_policy_ineffective",
|
||||
Self::HelperFirewallRuleCreateOrAddFailed => {
|
||||
"helper_firewall_rule_create_or_add_failed"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user