diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index aacbc6e1bf..e89497c3c5 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4622,6 +4622,7 @@ dependencies = [ "tokio", "tokio-util", "uds_windows", + "windows-sys 0.52.0", ] [[package]] diff --git a/codex-rs/app-server-transport/src/transport/unix_socket.rs b/codex-rs/app-server-transport/src/transport/unix_socket.rs index dd46d88280..bf976a3331 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket.rs @@ -1,3 +1,5 @@ +//! Control socket startup, guarded rendezvous paths, and WebSocket acceptance. + use std::fs::OpenOptions; use std::io::ErrorKind; use std::io::Result as IoResult; @@ -26,9 +28,21 @@ pub async fn start_control_socket_acceptor( transport_event_tx: mpsc::Sender, shutdown_token: CancellationToken, ) -> IoResult> { + #[cfg(windows)] + let (socket_path, directory_guard) = { + if let Some(parent) = socket_path.as_path().parent() { + codex_uds::prepare_private_socket_directory(parent).await?; + } + let (path, guard) = codex_uds::validate_private_socket_path(socket_path.as_path())?; + (AbsolutePathBuf::from_absolute_path_checked(path)?, guard) + }; prepare_control_socket_path(socket_path.as_path()).await?; let listener = UnixListener::bind(socket_path.as_path()).await?; - let socket_guard = ControlSocketFileGuard { socket_path }; + let socket_guard = ControlSocketFileGuard { + socket_path, + #[cfg(windows)] + _directory_guard: directory_guard, + }; set_control_socket_permissions(socket_guard.socket_path.as_path()).await?; info!( socket_path = %socket_guard.socket_path.display(), @@ -95,6 +109,13 @@ pub async fn prepare_control_socket_path(socket_path: &Path) -> IoResult<()> { codex_uds::prepare_private_socket_directory(parent).await?; } + #[cfg(windows)] + let (socket_path, _directory_guard) = codex_uds::validate_private_socket_path(socket_path)?; + #[cfg(windows)] + let socket_path = AbsolutePathBuf::from_absolute_path_checked(socket_path)?; + #[cfg(windows)] + let socket_path = socket_path.as_path(); + match UnixStream::connect(socket_path).await { Ok(_stream) => { return Err(std::io::Error::new( @@ -173,6 +194,9 @@ async fn set_control_socket_permissions(_socket_path: &Path) -> IoResult<()> { struct ControlSocketFileGuard { socket_path: AbsolutePathBuf, + // Keep the directory pinned until after the socket file is removed in Drop. + #[cfg(windows)] + _directory_guard: std::os::windows::io::OwnedHandle, } impl Drop for ControlSocketFileGuard { diff --git a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs index ac0b2b00c4..6d826be569 100644 --- a/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs +++ b/codex-rs/app-server-transport/src/transport/unix_socket_tests.rs @@ -190,6 +190,24 @@ async fn control_socket_file_is_private_after_bind() { accept_handle.await.expect("acceptor should join"); } +#[cfg(windows)] +#[tokio::test] +async fn control_socket_pins_directory_until_shutdown() { + let temp_dir = tempfile::TempDir::new().expect("temp dir"); + let socket_path = test_socket_path(temp_dir.path()); + let directory = socket_path.as_path().parent().unwrap(); + let moved = temp_dir.path().join("moved"); + let (tx, _rx) = mpsc::channel::(CHANNEL_CAPACITY); + let shutdown = CancellationToken::new(); + let acceptor = start_control_socket_acceptor(socket_path.clone(), tx, shutdown.clone()) + .await + .expect("acceptor"); + assert!(std::fs::rename(directory, &moved).is_err()); + shutdown.cancel(); + acceptor.await.expect("shutdown"); + std::fs::rename(directory, moved).expect("directory unpinned after cleanup"); +} + fn absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::from_absolute_path(path).expect("absolute path") } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index fbdb6bc12f..a7114a49ad 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -43,6 +43,11 @@ opens exactly one raw stream connection to `$CODEX_HOME/app-server-control/app-s by default, or to `--sock PATH` when provided, and proxies bytes between that socket and stdin/stdout. The proxied stream carries the websocket HTTP Upgrade handshake followed by websocket frames. +On Windows, the socket directory is created with a protected current-user-only DACL. Existing +directories must already have that owner and DACL; startup rejects broader permissions rather +than attempting to repair previously exposed state. Custom sockets should use a new dedicated +subdirectory. The listener pins the validated directory until socket cleanup completes. + Tracing/log output: - `RUST_LOG` controls log filtering/verbosity. diff --git a/codex-rs/app-server/tests/suite/v2/remote_control.rs b/codex-rs/app-server/tests/suite/v2/remote_control.rs index a8a843e3d0..17e0d2c5d4 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_control.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_control.rs @@ -213,7 +213,10 @@ async fn explicit_remote_control_startup_fails_when_disabled_by_requirements() - "allow_remote_control = false\n", )?; let managed_config_path = codex_home.path().join("managed_config.toml"); - let socket_path = codex_home.path().join("app-server.sock"); + let socket_path = codex_home + .path() + .join("app-server-control") + .join("app-server.sock"); let transport = AppServerTransport::from_listen_url(&format!("unix://{}", socket_path.display()))?; let _codex_home_guard = EnvVarGuard::set("CODEX_HOME", codex_home.path().as_os_str()); diff --git a/codex-rs/uds/Cargo.toml b/codex-rs/uds/Cargo.toml index 3477924772..9748c630b8 100644 --- a/codex-rs/uds/Cargo.toml +++ b/codex-rs/uds/Cargo.toml @@ -19,6 +19,7 @@ tokio = { workspace = true, features = ["fs", "net", "rt"] } async-io = { workspace = true } tokio-util = { workspace = true, features = ["compat"] } uds_windows = { workspace = true } +windows-sys = { version = "0.52", features = ["Win32_Foundation", "Win32_Security", "Win32_Security_Authorization", "Win32_Storage_FileSystem", "Win32_System_Threading", "Win32_System_SystemServices", "Win32_Networking_WinSock", "Win32_System_IO"] } [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/uds/src/lib.rs b/codex-rs/uds/src/lib.rs index bb6dd31bfc..a237d1060b 100644 --- a/codex-rs/uds/src/lib.rs +++ b/codex-rs/uds/src/lib.rs @@ -10,8 +10,10 @@ use tokio::io::AsyncRead; use tokio::io::AsyncWrite; use tokio::io::ReadBuf; -/// Creates `socket_dir` if needed and restricts it to the current user where -/// the platform exposes Unix permissions. +/// Creates `socket_dir` if needed and restricts it to the current user as +/// supported by the platform (0700 on Unix, a protected user-only DACL on Windows). +/// Windows rejects existing directories unless they already have that owner and DACL; +/// changing permissions cannot revoke access held through preexisting handles. pub async fn prepare_private_socket_directory(socket_dir: impl AsRef) -> IoResult<()> { platform::prepare_private_socket_directory(socket_dir.as_ref()).await } @@ -50,6 +52,13 @@ pub struct UnixStream { } impl UnixStream { + /// Requires the Windows peer to belong to the current user, with neither + /// process elevated. Call before sending any application data. + #[cfg(windows)] + pub fn ensure_non_elevated_peer(&self) -> IoResult<()> { + self.inner.ensure_non_elevated_peer() + } + /// Connects to `socket_path`. pub async fn connect(socket_path: impl AsRef) -> IoResult { platform::connect_stream(socket_path.as_ref()) @@ -184,8 +193,18 @@ mod platform { pub(super) struct Stream(Compat>); + impl Stream { + pub(super) fn ensure_non_elevated_peer(&self) -> IoResult<()> { + crate::windows_peer::ensure_non_elevated_peer( + self.0.get_ref().get_ref().as_raw_socket(), + ) + } + } + pub(super) async fn prepare_private_socket_directory(socket_dir: &Path) -> IoResult<()> { - tokio::fs::create_dir_all(socket_dir).await + let socket_dir = socket_dir.to_path_buf(); + spawn_blocking_io(move || crate::windows_security::prepare_private_directory(&socket_dir)) + .await } pub(super) struct Listener(Async); @@ -329,3 +348,14 @@ mod platform { #[cfg(test)] mod lib_tests; + +#[cfg(windows)] +mod windows_security; + +#[cfg(windows)] +mod windows_socket_validation; +#[cfg(windows)] +pub use windows_socket_validation::validate_private_socket_path; + +#[cfg(windows)] +mod windows_peer; diff --git a/codex-rs/uds/src/lib_tests.rs b/codex-rs/uds/src/lib_tests.rs index 1861cb5003..91040b1253 100644 --- a/codex-rs/uds/src/lib_tests.rs +++ b/codex-rs/uds/src/lib_tests.rs @@ -6,6 +6,168 @@ use tokio::io::AsyncWriteExt; use super::*; +#[cfg(windows)] +#[tokio::test] +async fn private_directory_rejects_volume_roots() { + // A nonexistent volume keeps this regression safe even if root validation breaks. + let root = Path::new(r"\\?\Volume{00000000-0000-0000-0000-000000000000}\"); + assert_eq!( + prepare_private_socket_directory(root) + .await + .unwrap_err() + .kind(), + ErrorKind::InvalidInput, + ); +} + +#[cfg(windows)] +#[tokio::test] +async fn private_directory_rejects_junctions() { + let temp = tempfile::TempDir::new().expect("temp directory"); + let target = temp.path().join("target"); + let junction = temp.path().join("junction"); + std::fs::create_dir(&target).expect("target"); + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", "$ErrorActionPreference = 'Stop'; New-Item -ItemType Junction -Path $env:CODEX_TEST_LINK -Target $env:CODEX_TEST_TARGET | Out-Null"]) + .env("CODEX_TEST_LINK", &junction) + .env("CODEX_TEST_TARGET", &target) + .output().expect("create junction"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + prepare_private_socket_directory(&junction) + .await + .unwrap_err() + .kind(), + ErrorKind::PermissionDenied, + ); + assert_eq!( + validate_private_socket_path(&junction.join("socket")) + .unwrap_err() + .kind(), + ErrorKind::PermissionDenied, + ); +} + +#[cfg(windows)] +#[tokio::test] +async fn socket_validation_pins_private_directory_without_creating_missing_paths() { + let temp = tempfile::TempDir::new().expect("temp directory"); + let directory = temp.path().join("private"); + let socket_path = directory.join("socket"); + assert!(validate_private_socket_path(&socket_path).is_err()); + assert!(!directory.exists()); + prepare_private_socket_directory(&directory).await.unwrap(); + let (_validated_path, _guard) = validate_private_socket_path(&socket_path).unwrap(); + assert!(std::fs::rename(&directory, temp.path().join("moved")).is_err()); +} + +#[cfg(windows)] +#[tokio::test] +async fn socket_validation_rejects_broad_acl_without_repairing_it() { + let temp = tempfile::TempDir::new().expect("temp directory"); + let directory = temp.path().join("private"); + prepare_private_socket_directory(&directory).await.unwrap(); + let inspect = |script: &str| { + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", script]) + .env("CODEX_TEST_DIRECTORY", &directory) + .output() + .unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + output.stdout + }; + let before = inspect( + r#" +$ErrorActionPreference = 'Stop' +$acl = Get-Acl -LiteralPath $env:CODEX_TEST_DIRECTORY +$everyone = [System.Security.Principal.SecurityIdentifier]::new('S-1-1-0') +$rule = [System.Security.AccessControl.FileSystemAccessRule]::new($everyone, 'FullControl', 'ContainerInherit,ObjectInherit', 'None', 'Allow') +$acl.AddAccessRule($rule) +Set-Acl -LiteralPath $env:CODEX_TEST_DIRECTORY -AclObject $acl +(Get-Acl -LiteralPath $env:CODEX_TEST_DIRECTORY).Sddl +"#, + ); + assert_eq!( + validate_private_socket_path(&directory.join("socket")) + .unwrap_err() + .kind(), + ErrorKind::PermissionDenied, + ); + assert_eq!( + prepare_private_socket_directory(&directory) + .await + .unwrap_err() + .kind(), + ErrorKind::PermissionDenied, + ); + assert_eq!( + before, + inspect( + "$ErrorActionPreference = 'Stop'; (Get-Acl -LiteralPath $env:CODEX_TEST_DIRECTORY).Sddl" + ) + ); +} + +#[cfg(windows)] +#[tokio::test] +async fn private_directory_supports_extended_length_paths() { + let temp = tempfile::TempDir::new().expect("temp directory"); + let directory = temp.path().join("a".repeat(150)).join("b".repeat(150)); + prepare_private_socket_directory(&directory) + .await + .expect("create long private directory"); + prepare_private_socket_directory(&directory) + .await + .expect("validate long private directory"); + assert!(directory.is_dir()); +} + +#[cfg(windows)] +#[tokio::test] +async fn private_directory_acl_is_user_only_and_inherited_by_files() { + let temp = tempfile::TempDir::new().expect("temp directory"); + let directory = temp.path().join("private"); + // Cover reusing an existing private directory as well as inheriting its DACL. + prepare_private_socket_directory(&directory) + .await + .expect("create user-owned directory"); + prepare_private_socket_directory(&directory) + .await + .expect("private directory"); + std::fs::write(directory.join("child"), b"").expect("child file"); + let _listener = UnixListener::bind(directory.join("socket")) + .await + .expect("private socket"); + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", r#" +$ErrorActionPreference = 'Stop' +$sid = [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value +$directory = Get-Acl -LiteralPath $env:CODEX_TEST_PRIVATE_DIRECTORY +if (-not $directory.AreAccessRulesProtected) { throw 'directory inherits broad access' } +foreach ($acl in @($directory, (Get-Acl -LiteralPath (Join-Path $env:CODEX_TEST_PRIVATE_DIRECTORY 'child')), (Get-Acl -LiteralPath (Join-Path $env:CODEX_TEST_PRIVATE_DIRECTORY 'socket')))) { + $rules = @($acl.Access) + if ($rules.Count -ne 1) { throw 'unexpected access rules' } + if ($rules[0].IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value -ne $sid) { throw 'wrong user' } + if ($rules[0].AccessControlType -ne 'Allow') { throw 'user is denied access' } +} +"#]) + .env("CODEX_TEST_PRIVATE_DIRECTORY", &directory) + .output().expect("inspect ACLs"); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); +} + #[tokio::test] async fn prepare_private_socket_directory_creates_directory() { let temp_dir = tempfile::TempDir::new().expect("temp dir"); @@ -119,3 +281,27 @@ async fn stream_round_trips_data_between_listener_and_client() { server_task.await.expect("server task should join"); } + +#[cfg(windows)] +#[tokio::test] +async fn implicit_peer_validation_rejects_elevated_listener() { + let temp = tempfile::TempDir::new().unwrap(); + let path = temp.path().join("socket"); + let mut listener = UnixListener::bind(&path).await.unwrap(); + let client = UnixStream::connect(&path).await.unwrap(); + let _server = listener.accept().await.unwrap(); + // PowerShell inherits our token and supplies an independent elevation oracle. + let output = std::process::Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", "([Security.Principal.WindowsPrincipal] [Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator)"]) + .output().unwrap(); + assert!(output.status.success()); + let expected = match String::from_utf8(output.stdout).unwrap().trim() { + "True" => Err(ErrorKind::PermissionDenied), + "False" => Ok(()), + other => panic!("unexpected elevation result: {other}"), + }; + assert_eq!( + client.ensure_non_elevated_peer().map_err(|err| err.kind()), + expected + ); +} diff --git a/codex-rs/uds/src/windows_peer.rs b/codex-rs/uds/src/windows_peer.rs new file mode 100644 index 0000000000..722133365b --- /dev/null +++ b/codex-rs/uds/src/windows_peer.rs @@ -0,0 +1,103 @@ +//! Authenticate implicitly discovered Windows socket peers before sending data. +//! The socket's kernel-reported PID, rather than a PID file, identifies the token. + +use std::io; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::os::windows::io::RawSocket; +use std::ptr; + +use windows_sys::Win32::Networking::WinSock::SIO_AF_UNIX_GETPEERPID; +use windows_sys::Win32::Networking::WinSock::SOCKET_ERROR; +use windows_sys::Win32::Networking::WinSock::WSAGetLastError; +use windows_sys::Win32::Networking::WinSock::WSAIoctl; +use windows_sys::Win32::Security::EqualSid; +use windows_sys::Win32::Security::GetTokenInformation; +use windows_sys::Win32::Security::TOKEN_ELEVATION; +use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::Security::TOKEN_USER; +use windows_sys::Win32::Security::TokenElevation; +use windows_sys::Win32::System::Threading::GetCurrentProcess; +use windows_sys::Win32::System::Threading::OpenProcess; +use windows_sys::Win32::System::Threading::OpenProcessToken; +use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; + +pub(super) fn ensure_non_elevated_peer(socket: RawSocket) -> io::Result<()> { + let mut pid = 0u32; + let mut returned = 0; + if unsafe { + WSAIoctl( + socket as _, + SIO_AF_UNIX_GETPEERPID, + ptr::null(), + /*cbinbuffer*/ 0, + ptr::addr_of_mut!(pid).cast(), + std::mem::size_of_val(&pid) as u32, + &mut returned, + ptr::null_mut(), + /*lpcompletionroutine*/ None, + ) + } == SOCKET_ERROR + { + return Err(io::Error::from_raw_os_error(unsafe { WSAGetLastError() })); + } + // Older Windows versions can return a valid PID with zero bytes reported. + if pid == 0 { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "socket peer PID unavailable", + )); + } + let process = unsafe { + OpenProcess( + PROCESS_QUERY_LIMITED_INFORMATION, + /*binherithandle*/ 0, + pid, + ) + }; + if process == 0 { + return Err(io::Error::last_os_error()); + } + let _process = unsafe { OwnedHandle::from_raw_handle(process as _) }; + let mut token = 0; + if unsafe { OpenProcessToken(process, TOKEN_QUERY, &mut token) } == 0 { + return Err(io::Error::last_os_error()); + } + let _token = unsafe { OwnedHandle::from_raw_handle(token as _) }; + let mut current_token = 0; + if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut current_token) } == 0 { + return Err(io::Error::last_os_error()); + } + let _current_token = unsafe { OwnedHandle::from_raw_handle(current_token as _) }; + let current_user = crate::windows_security::token_user(current_token)?; + let peer_user = crate::windows_security::token_user(token)?; + let same_user = unsafe { + EqualSid( + (*(current_user.as_ptr().cast::())).User.Sid, + (*(peer_user.as_ptr().cast::())).User.Sid, + ) != 0 + }; + // Implicit reuse must not change either side's elevation context. + for token in [current_token, token] { + let mut elevation: TOKEN_ELEVATION = unsafe { std::mem::zeroed() }; + if unsafe { + GetTokenInformation( + token, + TokenElevation, + ptr::addr_of_mut!(elevation).cast(), + std::mem::size_of_val(&elevation) as u32, + &mut returned, + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + if !same_user || elevation.TokenIsElevated != 0 { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "implicit daemon connection requires non-elevated current-user tokens", + )); + } + } + Ok(()) +} diff --git a/codex-rs/uds/src/windows_security.rs b/codex-rs/uds/src/windows_security.rs new file mode 100644 index 0000000000..9b818d8411 --- /dev/null +++ b/codex-rs/uds/src/windows_security.rs @@ -0,0 +1,128 @@ +//! Protect Windows socket rendezvous directories with an inheritable user-only +//! DACL at creation time. Reject unsafe existing directories instead of repairing them. + +use std::io; +use std::os::windows::ffi::OsStrExt; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::path::Path; +use std::ptr; + +use windows_sys::Win32::Foundation::HANDLE; +use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Security::Authorization::ConvertSidToStringSidW; +use windows_sys::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW; +use windows_sys::Win32::Security::GetTokenInformation; +use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; +use windows_sys::Win32::Security::TOKEN_QUERY; +use windows_sys::Win32::Security::TOKEN_USER; +use windows_sys::Win32::Security::TokenUser; +use windows_sys::Win32::Storage::FileSystem::CreateDirectoryW; +use windows_sys::Win32::System::Threading::GetCurrentProcess; +use windows_sys::Win32::System::Threading::OpenProcessToken; + +pub(super) fn prepare_private_directory(path: &Path) -> io::Result<()> { + let user = current_user()?; + let user_sid = unsafe { (*(user.as_ptr().cast::())).User.Sid }; + let mut sid_string = ptr::null_mut(); + if unsafe { ConvertSidToStringSidW(user_sid, &mut sid_string) } == 0 { + return Err(io::Error::last_os_error()); + } + let mut sid_length = 0; + while unsafe { *sid_string.add(sid_length) } != 0 { + sid_length += 1; + } + let sid = + String::from_utf16_lossy(unsafe { std::slice::from_raw_parts(sid_string, sid_length) }); + unsafe { LocalFree(sid_string as _) }; + let sddl: Vec = format!("O:{sid}D:P(A;OICI;FA;;;{sid})") + .encode_utf16() + .chain(Some(0)) + .collect(); + let mut descriptor = ptr::null_mut(); + if unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + /*stringsdrevision*/ 1, + &mut descriptor, + ptr::null_mut(), + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + let result = (|| { + // Canonicalize only the parent: this supplies Win32's extended-length + // prefix without following a reparse point at the directory being secured. + let absolute = std::path::absolute(path)?; + let path = match (absolute.parent(), absolute.file_name()) { + (Some(parent), Some(name)) => { + std::fs::create_dir_all(parent)?; + std::fs::canonicalize(parent)?.join(name) + } + _ => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "private directory must not be a filesystem root", + )); + } + }; + let path_wide: Vec = path.as_os_str().encode_wide().chain(Some(0)).collect(); + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + if unsafe { CreateDirectoryW(path_wide.as_ptr(), &attributes) } != 0 { + return Ok(()); + } + let err = io::Error::last_os_error(); + if err.kind() != io::ErrorKind::AlreadyExists { + return Err(err); + } + // Changing a DACL cannot revoke handles already held by another user. + // Existing directories must already meet our private-directory contract. + let _guard = crate::windows_socket_validation::validate_private_directory(&path)?; + Ok(()) + })(); + unsafe { LocalFree(descriptor as _) }; + result +} + +// Keep the pointer-aligned buffer alive while using its TOKEN_USER SID. +pub(super) fn current_user() -> io::Result> { + // Token information contains pointers, so allocate pointer-aligned storage. + let mut token = 0; + if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) } == 0 { + return Err(io::Error::last_os_error()); + } + let _token = unsafe { OwnedHandle::from_raw_handle(token as _) }; + token_user(token) +} + +pub(super) fn token_user(token: HANDLE) -> io::Result> { + let mut length = 0; + unsafe { + GetTokenInformation( + token, + TokenUser, + ptr::null_mut(), + /*tokeninformationlength*/ 0, + &mut length, + ) + }; + let mut user = vec![0usize; (length as usize).div_ceil(std::mem::size_of::())]; + if unsafe { + GetTokenInformation( + token, + TokenUser, + user.as_mut_ptr().cast(), + length, + &mut length, + ) + } == 0 + { + return Err(io::Error::last_os_error()); + } + Ok(user) +} diff --git a/codex-rs/uds/src/windows_socket_validation.rs b/codex-rs/uds/src/windows_socket_validation.rs new file mode 100644 index 0000000000..26a955bc28 --- /dev/null +++ b/codex-rs/uds/src/windows_socket_validation.rs @@ -0,0 +1,138 @@ +//! Read-only validation of Windows daemon rendezvous directories. Pin the +//! validated directory while connecting; never repair an untrusted listener's ACL. + +use std::io; +use std::os::windows::ffi::OsStrExt; +use std::os::windows::io::FromRawHandle; +use std::os::windows::io::OwnedHandle; +use std::path::Path; +use std::path::PathBuf; +use std::ptr; + +use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE; +use windows_sys::Win32::Security::ACE_HEADER; +use windows_sys::Win32::Security::Authorization::GetSecurityInfo; +use windows_sys::Win32::Security::Authorization::SE_FILE_OBJECT; +use windows_sys::Win32::Security::CONTAINER_INHERIT_ACE; +use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; +use windows_sys::Win32::Security::EqualSid; +use windows_sys::Win32::Security::GetAce; +use windows_sys::Win32::Security::GetSecurityDescriptorControl; +use windows_sys::Win32::Security::OBJECT_INHERIT_ACE; +use windows_sys::Win32::Security::OWNER_SECURITY_INFORMATION; +use windows_sys::Win32::Security::SE_DACL_PROTECTED; +use windows_sys::Win32::Security::TOKEN_USER; +use windows_sys::Win32::Storage::FileSystem::BY_HANDLE_FILE_INFORMATION; +use windows_sys::Win32::Storage::FileSystem::CreateFileW; +use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS; +use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_DIRECTORY; +use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; +use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; +use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_OPEN_REPARSE_POINT; +use windows_sys::Win32::Storage::FileSystem::FILE_LIST_DIRECTORY; +use windows_sys::Win32::Storage::FileSystem::FILE_READ_ATTRIBUTES; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; +use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE; +use windows_sys::Win32::Storage::FileSystem::GetFileInformationByHandle; +use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; +use windows_sys::Win32::Storage::FileSystem::READ_CONTROL; +use windows_sys::Win32::System::SystemServices::ACCESS_ALLOWED_ACE_TYPE; + +/// Validates an existing daemon socket directory without creating or changing it. +/// Connect using the returned path and retain the handle through connection setup +/// to prevent replacement of the validated directory. Ancestor junctions are +/// resolved before validation; the socket directory itself must not be a reparse point. +pub fn validate_private_socket_path(socket_path: &Path) -> io::Result<(PathBuf, OwnedHandle)> { + let absolute = std::path::absolute(socket_path)?; + let directory = absolute.parent().ok_or(io::ErrorKind::InvalidInput)?; + let socket_name = absolute.file_name().ok_or(io::ErrorKind::InvalidInput)?; + let (directory, guard) = validate_private_directory(directory)?; + Ok((directory.join(socket_name), guard)) +} + +pub(super) fn validate_private_directory(directory: &Path) -> io::Result<(PathBuf, OwnedHandle)> { + let parent = directory.parent().ok_or(io::ErrorKind::InvalidInput)?; + let directory_name = directory.file_name().ok_or(io::ErrorKind::InvalidInput)?; + let directory = std::fs::canonicalize(parent)?.join(directory_name); + let path_wide: Vec = directory.as_os_str().encode_wide().chain(Some(0)).collect(); + // Metadata-only access does not enforce sharing restrictions on Windows. + // Directory listing access makes omission of FILE_SHARE_DELETE pin the path. + let handle = unsafe { + CreateFileW( + path_wide.as_ptr(), + READ_CONTROL | FILE_READ_ATTRIBUTES | FILE_LIST_DIRECTORY, + FILE_SHARE_READ | FILE_SHARE_WRITE, + ptr::null(), + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + /*htemplatefile*/ 0, + ) + }; + if handle == INVALID_HANDLE_VALUE { + return Err(io::Error::last_os_error()); + } + let guard = unsafe { OwnedHandle::from_raw_handle(handle as _) }; + let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() }; + if unsafe { GetFileInformationByHandle(handle, &mut info) } == 0 { + return Err(io::Error::last_os_error()); + } + if info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY == 0 + || info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 + { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "untrusted socket directory", + )); + } + let user = crate::windows_security::current_user()?; + let user_sid = unsafe { (*(user.as_ptr().cast::())).User.Sid }; + let mut owner = ptr::null_mut(); + let mut dacl = ptr::null_mut(); + let mut descriptor = ptr::null_mut(); + let status = unsafe { + GetSecurityInfo( + handle, + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + &mut owner, + ptr::null_mut(), + &mut dacl, + ptr::null_mut(), + &mut descriptor, + ) + }; + if status != 0 { + return Err(io::Error::from_raw_os_error(status as i32)); + } + // Accept exactly the protected, inheritable user-only ACL set at daemon bind. + let trusted = unsafe { + let mut control = 0; + let mut revision = 0; + let mut ace = ptr::null_mut(); + !owner.is_null() + && EqualSid(owner, user_sid) != 0 + && GetSecurityDescriptorControl(descriptor, &mut control, &mut revision) != 0 + && control & SE_DACL_PROTECTED != 0 + && !dacl.is_null() + && (*dacl).AceCount == 1 + && GetAce(dacl, /*dwaceindex*/ 0, &mut ace) != 0 + && (*ace.cast::()).AceType == ACCESS_ALLOWED_ACE_TYPE as u8 + && (*ace.cast::()).AceFlags + == (CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE) as u8 + && (*ace.cast::()).Mask == FILE_ALL_ACCESS + && EqualSid( + ptr::addr_of_mut!((*ace.cast::()).SidStart).cast(), + user_sid, + ) != 0 + }; + unsafe { LocalFree(descriptor as _) }; + if !trusted { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "socket directory is not private to the current user", + )); + } + Ok((directory, guard)) +}