From 4fdf4c11131ec901a303f68e5ad8962469697bb6 Mon Sep 17 00:00:00 2001 From: johnl-oai Date: Wed, 2 Sep 2026 18:40:48 +0000 Subject: [PATCH] Add Windows sandbox client authentication (#42348) ## Why Sandbox provisioning IPC must authenticate callers before request handling can be enabled. ## What changed - Authorize named-pipe clients against the Codex package identity and keep the client process open through authentication. - Impersonate the caller and require its token to match the client process user, belong to an interactive named user, and not belong to the sandbox group. - Prepare and pin the requested Codex home, then validate machine policy with the impersonated token. - Allow known Codex package families when running an unpackaged debug service in foreground mode. Provisioning IPC remains disabled until this authentication is connected to the transport. ## Testing - Add a Windows test that verifies an unpackaged pipe client is rejected before it sends a provisioning request. GitOrigin-RevId: b0e87450c06f8de3eca6cb77daa9b11ef80699a9 --- codex-rs/windows-sandbox-service/src/ipc.rs | 4 + .../src/ipc/authentication.rs | 179 +++++++++++++++ .../windows-sandbox-service/src/ipc_tests.rs | 74 +++++++ codex-rs/windows-sandbox-service/src/lib.rs | 3 + .../src/package_identity.rs | 205 ++++++++++++++++++ .../windows-sandbox-service/src/service.rs | 1 + 6 files changed, 466 insertions(+) create mode 100644 codex-rs/windows-sandbox-service/src/ipc/authentication.rs create mode 100644 codex-rs/windows-sandbox-service/src/package_identity.rs diff --git a/codex-rs/windows-sandbox-service/src/ipc.rs b/codex-rs/windows-sandbox-service/src/ipc.rs index 70831447d4..7fc32e7b4b 100644 --- a/codex-rs/windows-sandbox-service/src/ipc.rs +++ b/codex-rs/windows-sandbox-service/src/ipc.rs @@ -2,12 +2,16 @@ // Keep these private until the authenticated transport is connected. #[allow(dead_code)] +mod authentication; +#[allow(dead_code)] mod home; #[allow(dead_code)] mod request; use anyhow::Result; #[cfg(test)] +use home::OwnedHandle; +#[cfg(test)] use home::pin_existing_ancestors; #[cfg(test)] use request::ProvisioningRequest; diff --git a/codex-rs/windows-sandbox-service/src/ipc/authentication.rs b/codex-rs/windows-sandbox-service/src/ipc/authentication.rs new file mode 100644 index 0000000000..64d48f7279 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/ipc/authentication.rs @@ -0,0 +1,179 @@ +//! Authenticates provisioning clients and validates impersonated machine policy. + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use std::ffi::c_void; +use std::mem::size_of; +use std::path::Path; +use std::path::PathBuf; +use std::ptr; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Security as security; +use windows_sys::Win32::System::Pipes as pipes; +use windows_sys::Win32::System::Threading as threading; + +use super::home::OwnedHandle; +use super::home::prepare_codex_home; +use super::request::ProvisioningRequest; + +pub(super) struct ClientIdentity { + pub(super) account: String, + pub(super) codex_home: PathBuf, + // Denying FILE_SHARE_DELETE pins every checked directory through provisioning. + _directory_handles: Vec, +} + +pub(super) fn authenticate_client( + pipe: HANDLE, + authorized_process: &crate::package_identity::AuthorizedClientProcess, + sandbox_sid: &[u8], + request: &ProvisioningRequest, +) -> Result<(ClientIdentity, Result<()>)> { + std::thread::scope(|scope| { + scope + .spawn(|| { + if unsafe { pipes::ImpersonateNamedPipeClient(pipe) } == 0 { + return Err(std::io::Error::last_os_error()) + .context("impersonate provisioning client"); + } + + let (identity, token) = authenticate_impersonated_client( + authorized_process, + sandbox_sid, + &request.codex_home, + )?; + let policy_result = crate::machine_policy::validate_provisioning_settings( + &identity.codex_home, + &request.settings, + &request.listeners, + token.0, + ); + Ok((identity, policy_result)) + }) + .join() + .map_err(|_| anyhow::anyhow!("provisioning client authentication thread panicked"))? + }) +} + +fn authenticate_impersonated_client( + authorized_process: &crate::package_identity::AuthorizedClientProcess, + sandbox_sid: &[u8], + requested_home: &Path, +) -> Result<(ClientIdentity, OwnedHandle)> { + let mut raw_token = 0; + if unsafe { + threading::OpenThreadToken( + threading::GetCurrentThread(), + security::TOKEN_QUERY | security::TOKEN_IMPERSONATE, + 1, + &mut raw_token, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("open provisioning client token"); + } + let token = OwnedHandle(raw_token); + crate::package_identity::authorize_client(authorized_process, token.0) + .context("authorize the packaged Codex provisioning client")?; + let mut session = 0_u32; + let mut returned = 0_u32; + if unsafe { + security::GetTokenInformation( + token.0, + security::TokenSessionId, + (&raw mut session).cast(), + size_of::() as u32, + &mut returned, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("read provisioning client session"); + } + if session == 0 { + bail!("non-interactive service accounts cannot request provisioning"); + } + + let mut is_sandbox_member = 0; + if unsafe { + security::CheckTokenMembership( + token.0, + sandbox_sid.as_ptr() as *mut c_void, + &mut is_sandbox_member, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("check sandbox group membership"); + } + if is_sandbox_member != 0 { + bail!("sandbox accounts cannot request provisioning"); + } + + let mut length = 0; + unsafe { + security::GetTokenInformation( + token.0, + security::TokenUser, + ptr::null_mut(), + 0, + &mut length, + ) + }; + if length < size_of::() as u32 { + return Err(std::io::Error::last_os_error()).context("size provisioning client identity"); + } + let mut user = vec![0_u8; length as usize]; + if unsafe { + security::GetTokenInformation( + token.0, + security::TokenUser, + user.as_mut_ptr().cast(), + length, + &mut length, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("read provisioning client identity"); + } + let sid = unsafe { ptr::read_unaligned(user.as_ptr().cast::()) } + .User + .Sid; + let account = account_name(sid)?; + let (codex_home, handles) = prepare_codex_home(requested_home)?; + Ok(( + ClientIdentity { + account, + codex_home, + _directory_handles: handles, + }, + token, + )) +} + +fn account_name(sid: *mut c_void) -> Result { + let mut name = [0_u16; 256]; + let mut domain = [0_u16; 256]; + let mut name_length = name.len() as u32; + let mut domain_length = domain.len() as u32; + let mut account_type: security::SID_NAME_USE = 0; + if unsafe { + security::LookupAccountSidW( + std::ptr::null(), + sid, + name.as_mut_ptr(), + &mut name_length, + domain.as_mut_ptr(), + &mut domain_length, + &mut account_type, + ) + } == 0 + { + return Err(std::io::Error::last_os_error()).context("resolve provisioning client name"); + } + if account_type != security::SidTypeUser || domain_length == 0 { + bail!("provisioning client is not a named Windows user"); + } + let name = String::from_utf16(&name[..name_length as usize])?; + let domain = String::from_utf16(&domain[..domain_length as usize])?; + Ok(format!("{domain}\\{name}")) +} diff --git a/codex-rs/windows-sandbox-service/src/ipc_tests.rs b/codex-rs/windows-sandbox-service/src/ipc_tests.rs index 9b353475f2..a30c11253a 100644 --- a/codex-rs/windows-sandbox-service/src/ipc_tests.rs +++ b/codex-rs/windows-sandbox-service/src/ipc_tests.rs @@ -1,3 +1,4 @@ +use super::OwnedHandle; use super::ProvisioningRequest; use super::pin_existing_ancestors; use super::validate_request; @@ -8,8 +9,16 @@ use codex_windows_sandbox::SandboxProvisioningRequest; use codex_windows_sandbox::SandboxProvisioningResponse; use codex_windows_sandbox::WindowsSandboxProvisioningSettings; use codex_windows_sandbox::WindowsSandboxProxyListeners; +use codex_windows_sandbox::to_wide; use codex_windows_sandbox::write_provisioning_frame; use std::path::PathBuf; +use std::ptr; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; +use windows_sys::Win32::Foundation as foundation; +use windows_sys::Win32::Storage::FileSystem as filesystem; +use windows_sys::Win32::Storage::Packaging::Appx; +use windows_sys::Win32::System::Pipes as pipes; fn framed_request(request: SandboxProvisioningRequest) -> Vec { let mut frame = Vec::new(); @@ -223,3 +232,68 @@ fn pin_existing_ancestors_accepts_drive_and_verbatim_drive_roots() { assert_eq!(handles.len(), 1); } } + +#[test] +fn unpackaged_pipe_clients_are_rejected_before_sending_a_request() { + let mut family_length = 0; + let package_status = + unsafe { Appx::GetCurrentPackageFamilyName(&mut family_length, ptr::null_mut()) }; + if package_status != foundation::APPMODEL_ERROR_NO_PACKAGE { + assert_eq!(package_status, foundation::ERROR_INSUFFICIENT_BUFFER); + return; + } + + static NEXT_PIPE_INSTANCE: AtomicU64 = AtomicU64::new(0); + let name = to_wide(format!( + r"\\.\pipe\OpenAI.CodexSandbox.Tests.{}.{}", + std::process::id(), + NEXT_PIPE_INSTANCE.fetch_add(1, Ordering::Relaxed) + )); + let server = unsafe { + pipes::CreateNamedPipeW( + name.as_ptr(), + filesystem::PIPE_ACCESS_DUPLEX | filesystem::FILE_FLAG_FIRST_PIPE_INSTANCE, + pipes::PIPE_TYPE_BYTE + | pipes::PIPE_READMODE_BYTE + | pipes::PIPE_WAIT + | pipes::PIPE_REJECT_REMOTE_CLIENTS, + 1, + 1024, + 1024, + 0, + ptr::null(), + ) + }; + assert_ne!(server, foundation::INVALID_HANDLE_VALUE); + let server = OwnedHandle(server); + + let client = unsafe { + filesystem::CreateFileW( + name.as_ptr(), + foundation::GENERIC_READ | foundation::GENERIC_WRITE, + 0, + ptr::null(), + filesystem::OPEN_EXISTING, + 0, + 0, + ) + }; + assert_ne!(client, foundation::INVALID_HANDLE_VALUE); + let _client = OwnedHandle(client); + + let connected = unsafe { pipes::ConnectNamedPipe(server.0, ptr::null_mut()) }; + assert!( + connected != 0 || unsafe { foundation::GetLastError() } == foundation::ERROR_PIPE_CONNECTED + ); + + let error = match crate::package_identity::authorize_client_process(server.0) { + Ok(_) => panic!("an unpackaged client was authorized without sending a request"), + Err(error) => error, + }; + assert!( + error + .to_string() + .contains("installed Codex package identity"), + "unexpected package authorization failure: {error:#}" + ); +} diff --git a/codex-rs/windows-sandbox-service/src/lib.rs b/codex-rs/windows-sandbox-service/src/lib.rs index 04dd1c2ea3..557d9826ca 100644 --- a/codex-rs/windows-sandbox-service/src/lib.rs +++ b/codex-rs/windows-sandbox-service/src/lib.rs @@ -7,6 +7,9 @@ mod ipc; #[allow(dead_code)] mod machine_policy; #[cfg(windows)] +#[allow(dead_code)] +mod package_identity; +#[cfg(windows)] mod service; #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/codex-rs/windows-sandbox-service/src/package_identity.rs b/codex-rs/windows-sandbox-service/src/package_identity.rs new file mode 100644 index 0000000000..5fbd790af9 --- /dev/null +++ b/codex-rs/windows-sandbox-service/src/package_identity.rs @@ -0,0 +1,205 @@ +//! Binds provisioning requests to the packaged Codex client and its Windows user. + +use std::io; +use std::mem::size_of; +use std::ptr; +#[cfg(debug_assertions)] +use std::sync::atomic::AtomicBool; +#[cfg(debug_assertions)] +use std::sync::atomic::Ordering; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use windows_sys::Win32::Foundation as foundation; +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Security as security; +use windows_sys::Win32::Storage::Packaging::Appx; +use windows_sys::Win32::System::Pipes; +use windows_sys::Win32::System::Threading; + +const MAX_PACKAGE_FAMILY_LENGTH: usize = 256; +const MAX_TOKEN_USER_BYTES: usize = 4096; + +#[cfg(debug_assertions)] +static FOREGROUND_MODE: AtomicBool = AtomicBool::new(false); + +struct OwnedHandle(HANDLE); + +pub(crate) struct AuthorizedClientProcess { + handle: OwnedHandle, +} + +impl Drop for OwnedHandle { + fn drop(&mut self) { + if self.0 != 0 && self.0 != foundation::INVALID_HANDLE_VALUE { + unsafe { foundation::CloseHandle(self.0) }; + } + } +} + +#[cfg(debug_assertions)] +pub(crate) fn enable_foreground_mode() { + FOREGROUND_MODE.store(true, Ordering::Release); +} + +pub(crate) fn authorize_client_process(pipe: HANDLE) -> Result { + let mut process_id = 0; + if unsafe { Pipes::GetNamedPipeClientProcessId(pipe, &mut process_id) } == 0 || process_id == 0 + { + return Err(io::Error::last_os_error()).context("identify the provisioning client process"); + } + + let process = unsafe { + Threading::OpenProcess(Threading::PROCESS_QUERY_LIMITED_INFORMATION, 0, process_id) + }; + if process == 0 { + return Err(io::Error::last_os_error()).context("open the provisioning client process"); + } + let process = OwnedHandle(process); + + let client_family = package_family_name( + |length, buffer| unsafe { Appx::GetPackageFamilyName(process.0, length, buffer) }, + "provisioning client", + )?; + + let service_family = package_family_name( + |length, buffer| unsafe { Appx::GetCurrentPackageFamilyName(length, buffer) }, + "provisioning service", + )?; + match client_family { + Some(client_family) => match service_family.as_deref() { + Some(service_family) if client_family != service_family => { + bail!("provisioning client package does not match the service package") + } + Some(_) => {} + #[cfg(debug_assertions)] + None if FOREGROUND_MODE.load(Ordering::Acquire) + && is_known_codex_package_family(&client_family) => {} + #[cfg(debug_assertions)] + None if FOREGROUND_MODE.load(Ordering::Acquire) => { + bail!("provisioning client does not belong to a trusted Codex package family") + } + None => bail!("provisioning service has no package identity"), + }, + None if service_family.is_some() => {} + #[cfg(debug_assertions)] + None if FOREGROUND_MODE.load(Ordering::Acquire) => {} + None => bail!("provisioning clients must run with an installed Codex package identity"), + } + + Ok(AuthorizedClientProcess { handle: process }) +} + +pub(crate) fn authorize_client( + process: &AuthorizedClientProcess, + client_token: HANDLE, +) -> Result<()> { + let mut process_token = 0; + if unsafe { + Threading::OpenProcessToken(process.handle.0, security::TOKEN_QUERY, &mut process_token) + } == 0 + { + return Err(io::Error::last_os_error()) + .context("open the provisioning client process token"); + } + let process_token = OwnedHandle(process_token); + let process_user = token_user(process_token.0).context("read the client process user")?; + let impersonated_user = + token_user(client_token).context("read the impersonated client user")?; + let process_sid = unsafe { + ptr::read_unaligned(process_user.as_ptr().cast::()) + .User + .Sid + }; + let impersonated_sid = unsafe { + ptr::read_unaligned(impersonated_user.as_ptr().cast::()) + .User + .Sid + }; + if process_sid.is_null() + || impersonated_sid.is_null() + || unsafe { security::EqualSid(process_sid, impersonated_sid) } == 0 + { + bail!("provisioning client process does not belong to the impersonated user"); + } + + Ok(()) +} + +fn package_family_name( + mut query: impl FnMut(*mut u32, *mut u16) -> u32, + subject: &str, +) -> Result> { + let mut length = 0; + let status = query(&mut length, ptr::null_mut()); + if status == foundation::APPMODEL_ERROR_NO_PACKAGE { + return Ok(None); + } + if status != foundation::ERROR_INSUFFICIENT_BUFFER { + return Err(io::Error::from_raw_os_error(status as i32)) + .with_context(|| format!("query the {subject} package family")); + } + if length == 0 || length as usize > MAX_PACKAGE_FAMILY_LENGTH { + bail!("the {subject} package family has an invalid length"); + } + + let mut buffer = vec![0_u16; length as usize]; + let status = query(&mut length, buffer.as_mut_ptr()); + if status != foundation::ERROR_SUCCESS { + return Err(io::Error::from_raw_os_error(status as i32)) + .with_context(|| format!("read the {subject} package family")); + } + + let value = buffer + .get(..length as usize) + .context("the package-family API returned an invalid length")?; + let Some((&0, value)) = value.split_last() else { + bail!("the {subject} package family is not null-terminated"); + }; + if value.is_empty() || value.contains(&0) { + bail!("the {subject} package family is malformed"); + } + Ok(Some( + String::from_utf16(value).context("the package family contains invalid UTF-16")?, + )) +} + +fn token_user(token: HANDLE) -> Result> { + let mut length = 0; + unsafe { + security::GetTokenInformation(token, security::TokenUser, ptr::null_mut(), 0, &mut length) + }; + if length < size_of::() as u32 || length as usize > MAX_TOKEN_USER_BYTES { + bail!("the Windows token returned an invalid user identity length"); + } + + let mut buffer = vec![0_u8; length as usize]; + if unsafe { + security::GetTokenInformation( + token, + security::TokenUser, + buffer.as_mut_ptr().cast(), + length, + &mut length, + ) + } == 0 + { + return Err(io::Error::last_os_error()).context("read the Windows token user"); + } + if length < size_of::() as u32 || length as usize > buffer.len() { + bail!("the Windows token returned a malformed user identity"); + } + Ok(buffer) +} + +#[cfg(debug_assertions)] +fn is_known_codex_package_family(package_family: &str) -> bool { + matches!( + package_family, + "OpenAI.Codex_3k8sg7r9htsxt" + | "OpenAI.CodexAlpha_3k8sg7r9htsxt" + | "OpenAI.CodexBeta_3k8sg7r9htsxt" + | "OpenAI.CodexNightly_3k8sg7r9htsxt" + ) +} diff --git a/codex-rs/windows-sandbox-service/src/service.rs b/codex-rs/windows-sandbox-service/src/service.rs index 993d7e1f86..29a1d118c0 100644 --- a/codex-rs/windows-sandbox-service/src/service.rs +++ b/codex-rs/windows-sandbox-service/src/service.rs @@ -87,6 +87,7 @@ pub(crate) fn run() -> Result<()> { #[cfg(debug_assertions)] pub(crate) fn run_foreground() -> Result<()> { + crate::package_identity::enable_foreground_mode(); crate::ipc::run(Arc::new(AtomicBool::new(false)), || { eprintln!("{SERVICE_NAME} listening on {}", crate::ipc::PIPE_NAME); Ok(())