mirror of
https://github.com/openai/codex.git
synced 2026-11-13 06:45:07 +00:00
Enable authenticated Windows sandbox provisioning (#42351)
## Why The Windows sandbox service had provisioning policy and client authentication in place, but its IPC listener was still disabled. ## What changed - Serve framed provisioning requests over a local named pipe, authenticate packaged clients, validate requests and machine policy, and report provisioning outcomes through bounded responses and Windows event logging. - Return `unavailable` for configuration parse failures so clients can fall back to the elevated setup helper. - Keep validated directory handles alive in the setup helper so path protections survive an unexpected service exit. - Make connection recovery and shutdown wakeups tolerate clients that disconnect before the listener accepts them. ## Testing Added Windows tests for response framing, configuration-error classification, pipe security and reconnect behavior, shutdown wakeups, and retained helper handles. GitOrigin-RevId: 68e9d546dcada88ff162ea4f98c0f4d748706b34
This commit is contained in:
1 parent
4fdf4c1113
commit
7e45bdb5fd
12 files changed
+857
-59
No files matched your search
@@ -126,6 +126,9 @@ mod setup;
|
||||
#[cfg(target_os = "windows")]
|
||||
mod setup_error;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
mod setup_launch;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
mod spawn_prep;
|
||||
|
||||
@@ -321,6 +324,8 @@ pub use setup::SetupRootOverrides;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::run_elevated_provisioning_setup;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::run_elevated_provisioning_setup_with_retained_handles;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::run_elevated_setup;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use setup::run_setup_refresh;
|
||||
|
||||
@@ -171,6 +171,9 @@ pub fn provision_windows_sandbox_via_service(
|
||||
crate::SandboxProvisioningResponse::Ok => {
|
||||
Ok(WindowsSandboxProvisioningOutcome::Provisioned)
|
||||
}
|
||||
crate::SandboxProvisioningResponse::Unavailable => {
|
||||
Ok(WindowsSandboxProvisioningOutcome::Unavailable)
|
||||
}
|
||||
crate::SandboxProvisioningResponse::Error { message } => Err(anyhow!(message)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,7 +55,11 @@ pub struct WindowsSandboxProxyListeners {
|
||||
#[serde(tag = "status", rename_all = "snake_case")]
|
||||
pub enum SandboxProvisioningResponse {
|
||||
Ok,
|
||||
Error { message: String },
|
||||
/// The client should fall back to its elevated setup helper.
|
||||
Unavailable,
|
||||
Error {
|
||||
message: String,
|
||||
},
|
||||
}
|
||||
|
||||
/// Write a length-prefixed provisioning-service message.
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::collections::BTreeSet;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::c_void;
|
||||
use std::os::windows::io::BorrowedHandle;
|
||||
use std::os::windows::process::CommandExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -897,6 +898,7 @@ fn run_setup_exe(
|
||||
payload: &ElevationPayload,
|
||||
needs_elevation: bool,
|
||||
codex_home: &Path,
|
||||
retained_handles: &[BorrowedHandle<'_>],
|
||||
) -> Result<()> {
|
||||
let payload_json = serde_json::to_string(payload).map_err(|err| {
|
||||
failure(
|
||||
@@ -905,8 +907,13 @@ fn run_setup_exe(
|
||||
)
|
||||
})?;
|
||||
let payload_b64 = BASE64_STANDARD.encode(payload_json.as_bytes());
|
||||
if !retained_handles.is_empty() {
|
||||
// Service requests are serialized and must not join a bare setup flight
|
||||
// whose helper was started without these directory protections.
|
||||
return run_setup_exe_payload(&payload_b64, needs_elevation, codex_home, retained_handles);
|
||||
}
|
||||
run_setup_singleflight(payload_b64.clone(), || {
|
||||
run_setup_exe_payload(&payload_b64, needs_elevation, codex_home)
|
||||
run_setup_exe_payload(&payload_b64, needs_elevation, codex_home, retained_handles)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -914,6 +921,7 @@ fn run_setup_exe_payload(
|
||||
payload_b64: &str,
|
||||
needs_elevation: bool,
|
||||
codex_home: &Path,
|
||||
retained_handles: &[BorrowedHandle<'_>],
|
||||
) -> Result<()> {
|
||||
use windows_sys::Win32::System::Threading::GetExitCodeProcess;
|
||||
use windows_sys::Win32::System::Threading::INFINITE;
|
||||
@@ -937,19 +945,27 @@ fn run_setup_exe_payload(
|
||||
};
|
||||
|
||||
if !needs_elevation {
|
||||
let status = Command::new(&exe)
|
||||
.arg(payload_b64)
|
||||
.creation_flags(0x08000000) // CREATE_NO_WINDOW
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.status()
|
||||
.map_err(|err| {
|
||||
failure(
|
||||
SetupErrorCode::OrchestratorHelperLaunchFailed,
|
||||
format!("failed to launch setup helper (non-elevated): {err}"),
|
||||
)
|
||||
})?;
|
||||
let status = if retained_handles.is_empty() {
|
||||
Command::new(&exe)
|
||||
.arg(payload_b64)
|
||||
.creation_flags(0x08000000) // CREATE_NO_WINDOW
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.status()
|
||||
} else {
|
||||
crate::setup_launch::spawn_with_retained_handles(
|
||||
Command::new(&exe).arg(payload_b64),
|
||||
retained_handles,
|
||||
)
|
||||
.and_then(|mut child| child.wait())
|
||||
}
|
||||
.map_err(|err| {
|
||||
failure(
|
||||
SetupErrorCode::OrchestratorHelperLaunchFailed,
|
||||
format!("failed to launch setup helper (non-elevated): {err}"),
|
||||
)
|
||||
})?;
|
||||
if !status.success() {
|
||||
return Err(report_helper_failure(
|
||||
codex_home,
|
||||
@@ -1052,7 +1068,7 @@ fn run_elevated_setup_inner(
|
||||
format!("failed to determine elevation state: {err}"),
|
||||
)
|
||||
})?;
|
||||
run_setup_exe(&payload, needs_elevation, request.codex_home)
|
||||
run_setup_exe(&payload, needs_elevation, request.codex_home, &[])
|
||||
}
|
||||
|
||||
fn elevated_provisioning_payload(
|
||||
@@ -1084,6 +1100,17 @@ pub fn run_elevated_provisioning_setup(
|
||||
codex_home: &Path,
|
||||
real_user: &str,
|
||||
settings: crate::WindowsSandboxProvisioningSettings,
|
||||
) -> Result<()> {
|
||||
run_elevated_provisioning_setup_with_retained_handles(codex_home, real_user, settings, &[])
|
||||
}
|
||||
|
||||
/// Runs service provisioning with directory protections retained by the helper
|
||||
/// itself, so they survive an unexpected exit of the provisioning service.
|
||||
pub fn run_elevated_provisioning_setup_with_retained_handles(
|
||||
codex_home: &Path,
|
||||
real_user: &str,
|
||||
settings: crate::WindowsSandboxProvisioningSettings,
|
||||
retained_handles: &[BorrowedHandle<'_>],
|
||||
) -> Result<()> {
|
||||
if !codex_home.is_absolute()
|
||||
|| !matches!(
|
||||
@@ -1138,7 +1165,12 @@ pub fn run_elevated_provisioning_setup(
|
||||
mode: SetupMode::ProvisionOnly,
|
||||
refresh_only: false,
|
||||
};
|
||||
run_setup_exe(&payload, /*needs_elevation*/ false, codex_home)
|
||||
run_setup_exe(
|
||||
&payload,
|
||||
/*needs_elevation*/ false,
|
||||
codex_home,
|
||||
retained_handles,
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn build_payload_roots(
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
//! Starts provisioning helpers only after their retained directory handles are installed.
|
||||
//! The child owns its duplicates until exit, independently of the service lifetime.
|
||||
|
||||
use std::io;
|
||||
use std::os::windows::io::AsRawHandle;
|
||||
use std::os::windows::io::BorrowedHandle;
|
||||
use std::os::windows::process::CommandExt;
|
||||
use std::process::Child;
|
||||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use windows_sys::Win32::Foundation::DUPLICATE_SAME_ACCESS;
|
||||
use windows_sys::Win32::Foundation::DuplicateHandle;
|
||||
use windows_sys::Win32::Foundation::HANDLE;
|
||||
use windows_sys::Win32::Foundation::NTSTATUS;
|
||||
use windows_sys::Win32::Foundation::RtlNtStatusToDosError;
|
||||
use windows_sys::Win32::System::Threading::CREATE_NO_WINDOW;
|
||||
use windows_sys::Win32::System::Threading::CREATE_SUSPENDED;
|
||||
use windows_sys::Win32::System::Threading::GetCurrentProcess;
|
||||
|
||||
#[link(name = "ntdll")]
|
||||
unsafe extern "system" {
|
||||
fn NtResumeProcess(process_handle: HANDLE) -> NTSTATUS;
|
||||
}
|
||||
|
||||
pub(super) fn spawn_with_retained_handles(
|
||||
command: &mut Command,
|
||||
retained_handles: &[BorrowedHandle<'_>],
|
||||
) -> io::Result<Child> {
|
||||
let mut child = command
|
||||
.creation_flags(CREATE_NO_WINDOW | CREATE_SUSPENDED)
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()?;
|
||||
let initialized = (|| {
|
||||
for handle in retained_handles {
|
||||
let mut child_handle = 0;
|
||||
if unsafe {
|
||||
DuplicateHandle(
|
||||
GetCurrentProcess(),
|
||||
handle.as_raw_handle() as HANDLE,
|
||||
child.as_raw_handle() as HANDLE,
|
||||
&mut child_handle,
|
||||
/*dwdesiredaccess*/ 0,
|
||||
/*binherithandle*/ 0,
|
||||
DUPLICATE_SAME_ACCESS,
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(io::Error::last_os_error());
|
||||
}
|
||||
// This value belongs to the child's handle table. Do not close it here
|
||||
// or make it inheritable by unrelated processes or helper descendants.
|
||||
}
|
||||
let status = unsafe { NtResumeProcess(child.as_raw_handle() as HANDLE) };
|
||||
if status < 0 {
|
||||
return Err(io::Error::from_raw_os_error(
|
||||
unsafe { RtlNtStatusToDosError(status) } as i32,
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
})();
|
||||
if let Err(error) = initialized {
|
||||
// A partially initialized helper must never run after its parent drops
|
||||
// the directory protections. Reap it, including any installed duplicates.
|
||||
child.kill()?;
|
||||
child.wait()?;
|
||||
return Err(error);
|
||||
}
|
||||
Ok(child)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "setup_launch_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,136 @@
|
||||
//! Exercises retained setup pins using bounded, disposable child processes.
|
||||
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::io::Write;
|
||||
use std::os::windows::fs::OpenOptionsExt;
|
||||
use std::os::windows::io::AsHandle;
|
||||
use std::path::PathBuf;
|
||||
use std::process::Child;
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use windows_sys::Win32::Storage::FileSystem::DELETE;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_DELETE_ON_CLOSE;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_READ_DATA;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
|
||||
|
||||
use super::spawn_with_retained_handles;
|
||||
|
||||
const CHILD_TEST: &str = "setup_launch::tests::retained_handles_child";
|
||||
const CHILD_DIRECTORY_ENV: &str = "CODEX_TEST_SETUP_LAUNCH_DIRECTORY";
|
||||
const CHILD_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 10);
|
||||
const POLL_INTERVAL: Duration = Duration::from_millis(/*millis*/ 10);
|
||||
|
||||
struct ChildGuard(Child);
|
||||
|
||||
impl Drop for ChildGuard {
|
||||
fn drop(&mut self) {
|
||||
let _ = self.0.kill();
|
||||
let _ = self.0.wait();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_retains_directory_pin_until_exit() -> io::Result<()> {
|
||||
let temporary = tempfile::tempdir()?;
|
||||
let directory = temporary.path().join("pinned");
|
||||
let renamed_directory = temporary.path().join("renamed");
|
||||
fs::create_dir(&directory)?;
|
||||
let pin = fs::OpenOptions::new()
|
||||
.read(true)
|
||||
.share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE)
|
||||
.custom_flags(FILE_FLAG_BACKUP_SEMANTICS)
|
||||
.open(&directory)?;
|
||||
let guard_path = directory.join(".guard");
|
||||
let guard = fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.access_mode(FILE_READ_DATA | DELETE)
|
||||
.share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE)
|
||||
.custom_flags(FILE_FLAG_DELETE_ON_CLOSE)
|
||||
.open(&guard_path)?;
|
||||
let mut command = Command::new(std::env::current_exe()?);
|
||||
command
|
||||
.args(["--exact", CHILD_TEST, "--ignored"])
|
||||
.env(CHILD_DIRECTORY_ENV, temporary.path());
|
||||
let mut child = ChildGuard(spawn_with_retained_handles(
|
||||
&mut command,
|
||||
&[pin.as_handle(), guard.as_handle()],
|
||||
)?);
|
||||
drop(pin);
|
||||
drop(guard);
|
||||
|
||||
let started = Instant::now();
|
||||
while !temporary.path().join("ready").exists() {
|
||||
assert_eq!(
|
||||
child.0.try_wait()?,
|
||||
None,
|
||||
"child exited before becoming ready"
|
||||
);
|
||||
assert!(
|
||||
started.elapsed() < CHILD_TIMEOUT,
|
||||
"child did not become ready"
|
||||
);
|
||||
std::thread::sleep(POLL_INTERVAL);
|
||||
}
|
||||
assert!(
|
||||
fs::rename(&directory, &renamed_directory).is_err(),
|
||||
"the child's duplicate must keep the directory pinned after the parent drops its handle"
|
||||
);
|
||||
assert_eq!(child.0.try_wait()?, None);
|
||||
assert!(guard_path.exists(), "the child must retain the guard file");
|
||||
|
||||
fs::write(temporary.path().join("release"), b"")?;
|
||||
let started = Instant::now();
|
||||
let status = loop {
|
||||
if let Some(status) = child.0.try_wait()? {
|
||||
break status;
|
||||
}
|
||||
assert!(started.elapsed() < CHILD_TIMEOUT, "child did not exit");
|
||||
std::thread::sleep(POLL_INTERVAL);
|
||||
};
|
||||
assert!(status.success(), "child failed: {status}");
|
||||
assert!(
|
||||
!guard_path.exists(),
|
||||
"the guard must be deleted on child exit"
|
||||
);
|
||||
fs::rename(&directory, &renamed_directory)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_executable_returns_spawn_error_without_closing_retained_handles() -> io::Result<()> {
|
||||
let temporary = tempfile::tempdir()?;
|
||||
let mut retained_file = tempfile::tempfile()?;
|
||||
let mut command = Command::new(temporary.path().join("missing.exe"));
|
||||
|
||||
let error = spawn_with_retained_handles(&mut command, &[retained_file.as_handle()])
|
||||
.expect_err("missing executable must fail to spawn");
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::NotFound);
|
||||
retained_file.write_all(b"the caller still owns its handle")?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "child process for child_retains_directory_pin_until_exit"]
|
||||
fn retained_handles_child() -> io::Result<()> {
|
||||
let Some(directory) = std::env::var_os(CHILD_DIRECTORY_ENV).map(PathBuf::from) else {
|
||||
return Ok(());
|
||||
};
|
||||
fs::write(directory.join("ready"), b"")?;
|
||||
let started = Instant::now();
|
||||
while !directory.join("release").exists() {
|
||||
assert!(
|
||||
started.elapsed() < CHILD_TIMEOUT,
|
||||
"parent did not release child"
|
||||
);
|
||||
std::thread::sleep(POLL_INTERVAL);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Reference in new issue
Block a user