mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Harden Windows control socket rendezvous (#42326)
## Why Windows control sockets need a rendezvous directory that cannot be accessed or replaced by another user while the listener is active. ## What changed - Create socket directories with a protected, inheritable, current-user-only DACL, and reject existing directories with broader permissions. - Validate socket paths without following a junction at the rendezvous directory, and pin the directory through listener cleanup to prevent replacement after validation. - Add a Windows peer check that verifies the kernel-reported peer process uses the current user's non-elevated token. - Document the Windows requirements for custom control socket paths. ## Testing Add Windows coverage for ACL inheritance and rejection, junction and volume-root rejection, extended-length paths, directory pinning, and elevated peer rejection. GitOrigin-RevId: cc8dd76e5732c288f6346cfd6138051188533bfa
This commit is contained in:
1 parent
e6ff749506
commit
73e94ee7a6
11 files changed
+642
-5
No files matched your search
@@ -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<TransportEvent>,
|
||||
shutdown_token: CancellationToken,
|
||||
) -> IoResult<JoinHandle<()>> {
|
||||
#[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 {
|
||||
|
||||
@@ -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::<TransportEvent>(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")
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user