better idempotency for creating/updating firewall rules during setup. (#8686)

make sure if the Sandbox has to re-initialize with different Sandbox
user SID, it still finds/updates the firewall rule instead of creating a
new one.
This commit is contained in:
iceweasel-oai
2026-01-05 10:42:33 -08:00
committed by GitHub
parent fabb797097
commit 720fa67816
2 changed files with 154 additions and 90 deletions

View File

@@ -0,0 +1,151 @@
#![cfg(target_os = "windows")]
use anyhow::Result;
use std::fs::File;
use std::io::Write;
use windows::core::Interface;
use windows::core::BSTR;
use windows::Win32::Foundation::VARIANT_TRUE;
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2;
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule3;
use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2;
use windows::Win32::NetworkManagement::WindowsFirewall::NetFwRule;
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_PROFILE2_ALL;
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_RULE_DIR_OUT;
use windows::Win32::System::Com::CoCreateInstance;
use windows::Win32::System::Com::CoInitializeEx;
use windows::Win32::System::Com::CoUninitialize;
use windows::Win32::System::Com::CLSCTX_INPROC_SERVER;
use windows::Win32::System::Com::COINIT_APARTMENTTHREADED;
// This is the stable identifier we use to find/update the rule idempotently.
// It intentionally does not change between installs.
const OFFLINE_BLOCK_RULE_NAME: &str = "codex_sandbox_offline_block_outbound";
// Friendly text shown in the firewall UI.
const OFFLINE_BLOCK_RULE_FRIENDLY: &str = "Codex Sandbox Offline - Block Outbound";
pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Result<()> {
let local_user_spec = format!("O:LSD:(A;;CC;;;{offline_sid})");
let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
if hr.is_err() {
return Err(anyhow::anyhow!("CoInitializeEx failed: {hr:?}"));
}
let result = unsafe {
(|| -> Result<()> {
let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER)
.map_err(|e| anyhow::anyhow!("CoCreateInstance NetFwPolicy2: {e:?}"))?;
let rules = policy
.Rules()
.map_err(|e| anyhow::anyhow!("INetFwPolicy2::Rules: {e:?}"))?;
// Block all outbound IP protocols for this user.
ensure_block_rule(
&rules,
OFFLINE_BLOCK_RULE_NAME,
OFFLINE_BLOCK_RULE_FRIENDLY,
NET_FW_IP_PROTOCOL_ANY.0,
&local_user_spec,
offline_sid,
log,
)?;
Ok(())
})()
};
unsafe {
CoUninitialize();
}
result
}
fn ensure_block_rule(
rules: &windows::Win32::NetworkManagement::WindowsFirewall::INetFwRules,
internal_name: &str,
friendly_desc: &str,
protocol: i32,
local_user_spec: &str,
offline_sid: &str,
log: &mut File,
) -> Result<()> {
let name = BSTR::from(internal_name);
let rule: INetFwRule3 = match unsafe { rules.Item(&name) } {
Ok(existing) => existing
.cast()
.map_err(|e| anyhow::anyhow!("cast existing firewall rule to INetFwRule3: {e:?}"))?,
Err(_) => {
let new_rule: INetFwRule3 =
unsafe { CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER) }
.map_err(|e| anyhow::anyhow!("CoCreateInstance NetFwRule: {e:?}"))?;
unsafe { new_rule.SetName(&name) }.map_err(|e| anyhow::anyhow!("SetName: {e:?}"))?;
// Set all properties before adding the rule so we don't leave half-configured rules.
configure_rule(
&new_rule,
friendly_desc,
protocol,
local_user_spec,
offline_sid,
)?;
unsafe { rules.Add(&new_rule) }.map_err(|e| anyhow::anyhow!("Rules::Add: {e:?}"))?;
new_rule
}
};
// Always re-apply fields to keep the setup idempotent.
configure_rule(&rule, friendly_desc, protocol, local_user_spec, offline_sid)?;
log_line(
log,
&format!(
"firewall rule configured name={internal_name} protocol={protocol} LocalUserAuthorizedList={local_user_spec}"
),
)?;
Ok(())
}
fn configure_rule(
rule: &INetFwRule3,
friendly_desc: &str,
protocol: i32,
local_user_spec: &str,
offline_sid: &str,
) -> Result<()> {
unsafe {
rule.SetDescription(&BSTR::from(friendly_desc))
.map_err(|e| anyhow::anyhow!("SetDescription: {e:?}"))?;
rule.SetDirection(NET_FW_RULE_DIR_OUT)
.map_err(|e| anyhow::anyhow!("SetDirection: {e:?}"))?;
rule.SetAction(NET_FW_ACTION_BLOCK)
.map_err(|e| anyhow::anyhow!("SetAction: {e:?}"))?;
rule.SetEnabled(VARIANT_TRUE)
.map_err(|e| anyhow::anyhow!("SetEnabled: {e:?}"))?;
rule.SetProfiles(NET_FW_PROFILE2_ALL.0)
.map_err(|e| anyhow::anyhow!("SetProfiles: {e:?}"))?;
rule.SetProtocol(protocol)
.map_err(|e| anyhow::anyhow!("SetProtocol: {e:?}"))?;
rule.SetLocalUserAuthorizedList(&BSTR::from(local_user_spec))
.map_err(|e| anyhow::anyhow!("SetLocalUserAuthorizedList: {e:?}"))?;
}
// Read-back verification: ensure we actually wrote the expected SID scope.
let actual = unsafe { rule.LocalUserAuthorizedList() }
.map_err(|e| anyhow::anyhow!("LocalUserAuthorizedList (read-back): {e:?}"))?;
let actual_str = actual.to_string();
if !actual_str.contains(offline_sid) {
anyhow::bail!(
"offline firewall rule user scope mismatch: expected SID {offline_sid}, got {actual_str}"
);
}
Ok(())
}
fn log_line(log: &mut File, msg: &str) -> Result<()> {
let ts = chrono::Utc::now().to_rfc3339();
writeln!(log, "[{ts}] {msg}")?;
Ok(())
}

View File

@@ -1,5 +1,7 @@
#![cfg(target_os = "windows")]
mod firewall;
use anyhow::Context;
use anyhow::Result;
use base64::engine::general_purpose::STANDARD as BASE64;
@@ -28,22 +30,6 @@ use std::path::PathBuf;
use std::process::Command;
use std::process::Stdio;
use std::sync::mpsc;
use windows::core::Interface;
use windows::core::BSTR;
use windows::Win32::Foundation::VARIANT_TRUE;
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2;
use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule3;
use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2;
use windows::Win32::NetworkManagement::WindowsFirewall::NetFwRule;
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_PROFILE2_ALL;
use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_RULE_DIR_OUT;
use windows::Win32::System::Com::CoCreateInstance;
use windows::Win32::System::Com::CoInitializeEx;
use windows::Win32::System::Com::CoUninitialize;
use windows::Win32::System::Com::CLSCTX_INPROC_SERVER;
use windows::Win32::System::Com::COINIT_APARTMENTTHREADED;
use windows_sys::Win32::Foundation::GetLastError;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Foundation::HLOCAL;
@@ -239,79 +225,6 @@ fn read_mask_allows_or_log(
}
}
fn run_netsh_firewall(sid: &str, log: &mut File) -> Result<()> {
let local_user_spec = format!("O:LSD:(A;;CC;;;{sid})");
let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
if hr.is_err() {
return Err(anyhow::anyhow!("CoInitializeEx failed: {hr:?}"));
}
let result = unsafe {
(|| -> Result<()> {
let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER)
.map_err(|e| anyhow::anyhow!("CoCreateInstance NetFwPolicy2: {e:?}"))?;
let rules = policy
.Rules()
.map_err(|e| anyhow::anyhow!("INetFwPolicy2::Rules: {e:?}"))?;
let name = BSTR::from("Codex Sandbox Offline - Block Outbound");
let rule: INetFwRule3 = match rules.Item(&name) {
Ok(existing) => existing.cast().map_err(|e| {
anyhow::anyhow!("cast existing firewall rule to INetFwRule3: {e:?}")
})?,
Err(_) => {
let new_rule: INetFwRule3 =
CoCreateInstance(&NetFwRule, None, CLSCTX_INPROC_SERVER)
.map_err(|e| anyhow::anyhow!("CoCreateInstance NetFwRule: {e:?}"))?;
new_rule
.SetName(&name)
.map_err(|e| anyhow::anyhow!("SetName: {e:?}"))?;
new_rule
.SetDirection(NET_FW_RULE_DIR_OUT)
.map_err(|e| anyhow::anyhow!("SetDirection: {e:?}"))?;
new_rule
.SetAction(NET_FW_ACTION_BLOCK)
.map_err(|e| anyhow::anyhow!("SetAction: {e:?}"))?;
new_rule
.SetEnabled(VARIANT_TRUE)
.map_err(|e| anyhow::anyhow!("SetEnabled: {e:?}"))?;
new_rule
.SetProfiles(NET_FW_PROFILE2_ALL.0)
.map_err(|e| anyhow::anyhow!("SetProfiles: {e:?}"))?;
new_rule
.SetProtocol(NET_FW_IP_PROTOCOL_ANY.0)
.map_err(|e| anyhow::anyhow!("SetProtocol: {e:?}"))?;
rules
.Add(&new_rule)
.map_err(|e| anyhow::anyhow!("Rules::Add: {e:?}"))?;
new_rule
}
};
rule.SetLocalUserAuthorizedList(&BSTR::from(local_user_spec.as_str()))
.map_err(|e| anyhow::anyhow!("SetLocalUserAuthorizedList: {e:?}"))?;
rule.SetEnabled(VARIANT_TRUE)
.map_err(|e| anyhow::anyhow!("SetEnabled: {e:?}"))?;
rule.SetProfiles(NET_FW_PROFILE2_ALL.0)
.map_err(|e| anyhow::anyhow!("SetProfiles: {e:?}"))?;
rule.SetAction(NET_FW_ACTION_BLOCK)
.map_err(|e| anyhow::anyhow!("SetAction: {e:?}"))?;
rule.SetDirection(NET_FW_RULE_DIR_OUT)
.map_err(|e| anyhow::anyhow!("SetDirection: {e:?}"))?;
rule.SetProtocol(NET_FW_IP_PROTOCOL_ANY.0)
.map_err(|e| anyhow::anyhow!("SetProtocol: {e:?}"))?;
log_line(
log,
&format!(
"firewall rule configured via COM with LocalUserAuthorizedList={local_user_spec}"
),
)?;
Ok(())
})()
};
unsafe {
CoUninitialize();
}
result
}
fn lock_sandbox_dir(
dir: &Path,
real_user: &str,
@@ -549,7 +462,7 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
};
let mut refresh_errors: Vec<String> = Vec::new();
if !refresh_only {
run_netsh_firewall(&offline_sid_str, log)?;
firewall::ensure_offline_outbound_block(&offline_sid_str, log)?;
}
if payload.read_roots.is_empty() {