Skip to content

Commit

Permalink
Update windows crate dependency to 0.57 (#90)
Browse files Browse the repository at this point in the history
As it happens, another update to the `windows` family of crates is available:
https://github.com/microsoft/windows-rs/releases/tag/0.57.0

In particular, this one includes a new feature to support freeing handles automatically:
microsoft/windows-rs#3013

This avoids the need for the `OwnedHandle` helper type in Sudo.
  • Loading branch information
kennykerr authored Jul 1, 2024
1 parent 0082369 commit bb1adf9
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 55 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ embed-manifest = "1.4"
which = "6.0"
win_etw_provider = "0.1.8"
win_etw_macros = "0.1.8"
windows = "0.56"
windows = "0.57"
windows-registry = "0.1"
winres = "0.1"

Expand Down
4 changes: 2 additions & 2 deletions sudo/src/elevate_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn spawn_target_for_request(request: &ElevateRequest) -> Result<std::process
/// * Conditionally attach to the parent process's console (if requested)
/// * Spawn the target process (with redirected input/output if requested, and with the environment variables passed in if needed)
/// Called by rust_handle_elevation_request
pub fn handle_elevation_request(request: &ElevateRequest) -> Result<OwnedHandle> {
pub fn handle_elevation_request(request: &ElevateRequest) -> Result<Owned<HANDLE>> {
// Log the request we received to the event log. This should create a pair
// of events, one for the request, and one for the response, each with the
// same RequestID.
Expand Down Expand Up @@ -135,7 +135,7 @@ pub fn handle_elevation_request(request: &ElevateRequest) -> Result<OwnedHandle>
// in the COM API to have it limit the handle permissions but that didn't work at all.
// So now we do it manually here.
unsafe {
let mut child_handle = OwnedHandle::default();
let mut child_handle = Owned::default();
let current_process = GetCurrentProcess();
DuplicateHandle(
current_process,
Expand Down
46 changes: 9 additions & 37 deletions sudo/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,38 +62,6 @@ impl From<SudoMode> for i32 {
}
}

#[repr(transparent)]
#[derive(Default)]
pub struct OwnedHandle(pub HANDLE);

impl OwnedHandle {
pub fn new(handle: HANDLE) -> Self {
OwnedHandle(handle)
}
}

impl Drop for OwnedHandle {
fn drop(&mut self) {
if !self.0.is_invalid() {
unsafe { _ = CloseHandle(self.0) }
}
}
}

impl Deref for OwnedHandle {
type Target = HANDLE;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl DerefMut for OwnedHandle {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

// There can be many different types that need to be LocalFree'd. PWSTR, PCWSTR, PSTR, PCSTR, PSECURITY_DESCRIPTOR
// are all distinct types, but they are compatible with the windows::core::IntoParam<HLOCAL> trait.
// There's also *mut ACL though which is also LocalAlloc'd and that's the problem (probably not the last of its kind).
Expand Down Expand Up @@ -159,15 +127,15 @@ pub fn is_running_elevated() -> Result<bool> {
Ok(elevation.TokenIsElevated == 1)
}

fn current_process_token() -> Result<OwnedHandle> {
let mut token: OwnedHandle = Default::default();
fn current_process_token() -> Result<Owned<HANDLE>> {
let mut token = Owned::default();
unsafe {
OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut *token)?;
}
Ok(token)
}
fn get_process_token(process: HANDLE) -> Result<OwnedHandle> {
let mut token: OwnedHandle = Default::default();
fn get_process_token(process: HANDLE) -> Result<Owned<HANDLE>> {
let mut token = Owned::default();
unsafe {
OpenProcessToken(process, TOKEN_QUERY, &mut *token)?;
}
Expand Down Expand Up @@ -305,7 +273,11 @@ pub fn env_as_string() -> String {

// Try to figure out the end of the double-null terminated env block.
loop {
let len = wcslen(PCWSTR(end));
extern "C" {
fn wcslen(s: *const u16) -> usize;
}

let len = wcslen(end);
if len == 0 {
break;
}
Expand Down
6 changes: 3 additions & 3 deletions sudo/src/rpc_bindings_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ unsafe extern "system" fn rpc_server_callback(
_interface_uuid: *const c_void,
context: *const c_void,
) -> RPC_STATUS {
let mut client_handle = OwnedHandle::default();
let mut client_handle = Owned::default();
let status = I_RpcOpenClientProcess(
Some(context),
PROCESS_QUERY_LIMITED_INFORMATION.0,
Expand Down Expand Up @@ -80,7 +80,7 @@ fn create_security_descriptor_for_process(pid: u32) -> Result<OwnedSecurityDescr
{
let user = {
let process =
OwnedHandle::new(OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, pid)?);
Owned::new(OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, pid)?);
get_sid_for_process(*process)?
};

Expand Down Expand Up @@ -227,7 +227,7 @@ pub extern "C" fn server_DoElevationRequest(

match result {
Ok(mut handle) => {
unsafe { child.write(take(&mut handle.0)) };
unsafe { child.write(take(&mut handle)) };
HRESULT::default()
}
Err(err) => err.into(),
Expand Down
8 changes: 4 additions & 4 deletions sudo/src/run_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ fn adjust_args_for_intrinsics_and_cmdlets(req: &mut ElevateRequest) -> Result<bo
};
tracing::trace_log_message(&format!("parent_pid: {parent_pid:?}"));
// Now, open that process so we can query some more information about it.
// (stick it in an OwnedHandle so it gets closed when we're done with it)
// (stick it in an `Owned<T>` so it gets closed when we're done with it)
let parent_process_handle = unsafe {
OwnedHandle::new(OpenProcess(
Owned::new(OpenProcess(
PROCESS_QUERY_LIMITED_INFORMATION,
false,
parent_pid.try_into().unwrap(),
Expand Down Expand Up @@ -417,7 +417,7 @@ fn send_request_via_rpc(req: &ElevateRequest) -> Result<i32> {
// The GetCurrentProcess() is not a "real" handle and unsuitable to be used with COM.
// -> We need to clone it first.
let h_real = unsafe {
let mut process = OwnedHandle::default();
let mut process = Owned::default();
let current_process = GetCurrentProcess();
DuplicateHandle(
current_process,
Expand All @@ -433,7 +433,7 @@ fn send_request_via_rpc(req: &ElevateRequest) -> Result<i32> {

tracing::trace_log_message(&format!("sending i/o/e handles: {:?}", req.handles));

let mut child_handle = OwnedHandle::default();
let mut child_handle = Owned::default();
let rpc_elevate = rpc_client_do_elevation_request(
*h_real,
&req.handles,
Expand Down

0 comments on commit bb1adf9

Please sign in to comment.