mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Restrict Windows IDE pipe client impersonation (#39020)
## Why Windows named pipe servers can impersonate connected clients. IDE context connections only require the server to identify the client, so they should not grant broader impersonation rights. ## What changed Open Windows IDE context pipes with `SECURITY_SQOS_PRESENT` and `SECURITY_IDENTIFICATION`, limiting the server's impersonation token to the identification level. ## Testing Add a Windows regression test that connects to a named pipe, asks the server to impersonate the client, and verifies that the resulting token has the `SecurityIdentification` level. GitOrigin-RevId: 51ef080f3367857987e03b0113a476040ea40e56
This commit is contained in:
@@ -31,6 +31,8 @@ use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_WRITE;
|
||||
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
|
||||
use windows_sys::Win32::Storage::FileSystem::ReadFile;
|
||||
use windows_sys::Win32::Storage::FileSystem::SECURITY_IDENTIFICATION;
|
||||
use windows_sys::Win32::Storage::FileSystem::SECURITY_SQOS_PRESENT;
|
||||
use windows_sys::Win32::Storage::FileSystem::WriteFile;
|
||||
use windows_sys::Win32::System::IO::CancelIoEx;
|
||||
use windows_sys::Win32::System::IO::GetOverlappedResult;
|
||||
@@ -67,7 +69,10 @@ impl WindowsPipeStream {
|
||||
FILE_SHARE_READ | FILE_SHARE_WRITE,
|
||||
ptr::null(),
|
||||
OPEN_EXISTING,
|
||||
FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OVERLAPPED,
|
||||
FILE_ATTRIBUTE_NORMAL
|
||||
| FILE_FLAG_OVERLAPPED
|
||||
| SECURITY_SQOS_PRESENT
|
||||
| SECURITY_IDENTIFICATION,
|
||||
NULL_HANDLE,
|
||||
)
|
||||
};
|
||||
@@ -337,3 +342,7 @@ fn remaining_timeout_ms(deadline: Instant) -> u32 {
|
||||
fn timeout_io_error() -> io::Error {
|
||||
io::Error::new(io::ErrorKind::TimedOut, "timed out waiting for IDE context")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "windows_pipe_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,99 @@
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::time::Duration;
|
||||
use windows_sys::Win32::Foundation::ERROR_PIPE_CONNECTED;
|
||||
use windows_sys::Win32::Security::RevertToSelf;
|
||||
use windows_sys::Win32::Security::SecurityIdentification;
|
||||
use windows_sys::Win32::Security::TokenImpersonationLevel;
|
||||
use windows_sys::Win32::Storage::FileSystem::PIPE_ACCESS_DUPLEX;
|
||||
use windows_sys::Win32::System::Pipes::ConnectNamedPipe;
|
||||
use windows_sys::Win32::System::Pipes::CreateNamedPipeW;
|
||||
use windows_sys::Win32::System::Pipes::ImpersonateNamedPipeClient;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_READMODE_BYTE;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_TYPE_BYTE;
|
||||
use windows_sys::Win32::System::Pipes::PIPE_WAIT;
|
||||
use windows_sys::Win32::System::Threading::GetCurrentThread;
|
||||
use windows_sys::Win32::System::Threading::OpenThreadToken;
|
||||
|
||||
#[test]
|
||||
fn pipe_server_cannot_impersonate_client() {
|
||||
let pipe_path = PathBuf::from(format!(
|
||||
r"\\.\pipe\codex-ide-context-{}",
|
||||
uuid::Uuid::new_v4()
|
||||
));
|
||||
let wide_path = pipe_path
|
||||
.as_os_str()
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect::<Vec<_>>();
|
||||
let pipe = unsafe {
|
||||
CreateNamedPipeW(
|
||||
wide_path.as_ptr(),
|
||||
PIPE_ACCESS_DUPLEX,
|
||||
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
|
||||
/*nmaxinstances*/ 1,
|
||||
/*noutbuffersize*/ 1024,
|
||||
/*ninbuffersize*/ 1024,
|
||||
/*ndefaulttimeout*/ 0,
|
||||
ptr::null(),
|
||||
)
|
||||
};
|
||||
assert_ne!(pipe, INVALID_HANDLE_VALUE);
|
||||
let pipe = OwnedHandle(pipe);
|
||||
|
||||
let server = std::thread::spawn(move || {
|
||||
if unsafe { ConnectNamedPipe(pipe.raw(), ptr::null_mut()) } == FALSE {
|
||||
assert_eq!(
|
||||
io::Error::last_os_error().raw_os_error(),
|
||||
Some(ERROR_PIPE_CONNECTED as i32)
|
||||
);
|
||||
}
|
||||
|
||||
let mut message = [0_u8];
|
||||
let mut bytes_read = 0;
|
||||
assert_ne!(
|
||||
unsafe {
|
||||
ReadFile(
|
||||
pipe.raw(),
|
||||
message.as_mut_ptr(),
|
||||
message.len() as u32,
|
||||
&mut bytes_read,
|
||||
ptr::null_mut(),
|
||||
)
|
||||
},
|
||||
FALSE
|
||||
);
|
||||
assert_ne!(unsafe { ImpersonateNamedPipeClient(pipe.raw()) }, FALSE);
|
||||
|
||||
let mut token = NULL_HANDLE;
|
||||
assert_ne!(
|
||||
unsafe { OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, TRUE, &mut token) },
|
||||
FALSE
|
||||
);
|
||||
let token = OwnedHandle(token);
|
||||
let mut impersonation_level = 0;
|
||||
let mut return_length = 0;
|
||||
let result = unsafe {
|
||||
GetTokenInformation(
|
||||
token.raw(),
|
||||
TokenImpersonationLevel,
|
||||
(&raw mut impersonation_level).cast(),
|
||||
std::mem::size_of_val(&impersonation_level) as u32,
|
||||
&mut return_length,
|
||||
)
|
||||
};
|
||||
let reverted = unsafe { RevertToSelf() };
|
||||
assert_ne!(result, FALSE);
|
||||
assert_ne!(reverted, FALSE);
|
||||
impersonation_level
|
||||
});
|
||||
|
||||
let deadline = Instant::now() + Duration::from_secs(/*secs*/ 5);
|
||||
let mut stream = WindowsPipeStream::connect(pipe_path, deadline).expect("connect pipe client");
|
||||
stream.write_all(&[1]).expect("write pipe message");
|
||||
|
||||
assert_eq!(
|
||||
server.join().expect("join pipe server"),
|
||||
SecurityIdentification
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user