From 43fa973f2d06d6e104bfd5c5d6352007723427ae Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Mon, 13 Apr 2026 11:46:52 -0700 Subject: [PATCH] only specify remote ports when the rule needs them --- codex-rs/windows-sandbox-rs/src/firewall.rs | 117 ++++++++++++++++---- 1 file changed, 95 insertions(+), 22 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/firewall.rs b/codex-rs/windows-sandbox-rs/src/firewall.rs index cfd114f648e..b5dfb2ef496 100644 --- a/codex-rs/windows-sandbox-rs/src/firewall.rs +++ b/codex-rs/windows-sandbox-rs/src/firewall.rs @@ -303,28 +303,7 @@ fn configure_rule(rule: &INetFwRule3, spec: &BlockRuleSpec<'_>) -> Result<()> { format!("SetProfiles failed: {err:?}"), )) })?; - rule.SetProtocol(spec.protocol).map_err(|err| { - anyhow::Error::new(SetupFailure::new( - SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, - format!("SetProtocol failed: {err:?}"), - )) - })?; - let remote_addresses = spec.remote_addresses.unwrap_or("*"); - rule.SetRemoteAddresses(&BSTR::from(remote_addresses)) - .map_err(|err| { - anyhow::Error::new(SetupFailure::new( - SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, - format!("SetRemoteAddresses failed: {err:?}"), - )) - })?; - let remote_ports = spec.remote_ports.unwrap_or("*"); - rule.SetRemotePorts(&BSTR::from(remote_ports)) - .map_err(|err| { - anyhow::Error::new(SetupFailure::new( - SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, - format!("SetRemotePorts failed: {err:?}"), - )) - })?; + configure_rule_network_scope(rule, spec)?; rule.SetLocalUserAuthorizedList(&BSTR::from(spec.local_user_spec)) .map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -354,6 +333,36 @@ fn configure_rule(rule: &INetFwRule3, spec: &BlockRuleSpec<'_>) -> Result<()> { Ok(()) } +fn configure_rule_network_scope(rule: &INetFwRule3, spec: &BlockRuleSpec<'_>) -> Result<()> { + unsafe { + rule.SetProtocol(spec.protocol).map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetProtocol failed: {err:?}"), + )) + })?; + let remote_addresses = spec.remote_addresses.unwrap_or("*"); + rule.SetRemoteAddresses(&BSTR::from(remote_addresses)) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetRemoteAddresses failed: {err:?}"), + )) + })?; + if let Some(remote_ports) = spec.remote_ports { + rule.SetRemotePorts(&BSTR::from(remote_ports)) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetRemotePorts failed: {err:?}"), + )) + })?; + } + } + + Ok(()) +} + fn blocked_loopback_tcp_remote_ports(proxy_ports: &[u16]) -> Option { let mut allowed_ports = proxy_ports .iter() @@ -436,4 +445,68 @@ mod tests { ); } } + + #[test] + fn production_firewall_rule_network_scopes_are_accepted_by_firewall_com() { + let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; + assert!(hr.is_ok(), "CoInitializeEx failed: {hr:?}"); + + let local_user_spec = "O:LSD:(A;;CC;;;S-1-5-18)"; + let offline_sid = "S-1-5-18"; + let blocked_remote_ports = + blocked_loopback_tcp_remote_ports(&[8080]).expect("proxy-port complement should exist"); + let specs = [ + BlockRuleSpec { + internal_name: OFFLINE_BLOCK_LOOPBACK_UDP_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_LOOPBACK_UDP_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_UDP.0, + local_user_spec, + offline_sid, + remote_addresses: Some(LOOPBACK_REMOTE_ADDRESSES), + remote_ports: None, + }, + BlockRuleSpec { + internal_name: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_TCP.0, + local_user_spec, + offline_sid, + remote_addresses: Some(LOOPBACK_REMOTE_ADDRESSES), + remote_ports: Some(&blocked_remote_ports), + }, + BlockRuleSpec { + internal_name: OFFLINE_BLOCK_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_ANY.0, + local_user_spec, + offline_sid, + remote_addresses: Some(NON_LOOPBACK_REMOTE_ADDRESSES), + remote_ports: None, + }, + ]; + + let results = specs.each_ref().map(|spec| unsafe { + let rule: windows::core::Result = + CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER); + match rule { + Ok(rule) => configure_rule_network_scope(&rule, spec), + Err(err) => Err(err.into()), + } + }); + + unsafe { + CoUninitialize(); + } + + for (spec, result) in specs.into_iter().zip(results) { + assert!( + result.is_ok(), + "firewall rejected network scope for rule={} protocol={} remote_addresses={:?} remote_ports={:?}: {result:?}", + spec.internal_name, + spec.protocol, + spec.remote_addresses, + spec.remote_ports + ); + } + } }