From 0dfdedcf2453f7a99a5bc0e8f7d4cc0eaa945860 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 3 Jul 2024 10:57:42 -0500 Subject: [PATCH 1/4] everything is awful --- .cargo/config.toml | 5 +++++ .cargo/ms-toolchain-config.toml | 4 ++-- Building.md | 13 +++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index b4178fb..12854ce 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -15,3 +15,8 @@ rustflags = ["-Clink-args=/DEFAULTLIB:rpcrt4.lib"] # -Clink-args=/DYNAMICBASE /CETCOMPAT: Enable "shadow stack" (https://learn.microsoft.com/en-us/cpp/build/reference/cetcompat) [target.'cfg(all(target_os = "windows", any(target_arch = "x86", target_arch = "x86_64")))'] rustflags = ["-Clink-args=/DYNAMICBASE /CETCOMPAT"] + +[registries] +Sudo_PublicPackages = { index = "sparse+https://pkgs.dev.azure.com/shine-oss/sudo/_packaging/Sudo_PublicPackages/Cargo/index/" } +[source.crates-io] +replace-with = "Sudo_PublicPackages" diff --git a/.cargo/ms-toolchain-config.toml b/.cargo/ms-toolchain-config.toml index e4a24ca..c0ede21 100644 --- a/.cargo/ms-toolchain-config.toml +++ b/.cargo/ms-toolchain-config.toml @@ -20,6 +20,6 @@ rustflags = ["-Clink-args=/DYNAMICBASE /CETCOMPAT"] # Setup the ADO Artifacts feed as a Registry: you'll need to use your own feed in your project that upstreams from crates.io. # For more details see https://eng.ms/docs/cloud-ai-platform/devdiv/one-engineering-system-1es/1es-docs/azure-artifacts/cargo [registries] -sudo_public_cargo = { index = "sparse+https://pkgs.dev.azure.com/microsoft/Dart/_packaging/sudo_public_cargo/Cargo/index/" } +Sudo_PublicPackages = { index = "sparse+https://pkgs.dev.azure.com/shine-oss/sudo/_packaging/Sudo_PublicPackages/Cargo/index/" } [source.crates-io] -replace-with = "sudo_public_cargo" +replace-with = "Sudo_PublicPackages" diff --git a/Building.md b/Building.md index 5ad8e8e..37d72ff 100644 --- a/Building.md +++ b/Building.md @@ -73,3 +73,16 @@ cargo build --config .cargo\ms-toolchain-config.toml ``` Note, if you run that on the public toolchain, you'll most definitely run into ``error: unknown codegen option: `ehcont_guard` `` when building. + +### Notes on updating the cargo feed + +For internal reasons, we need to maintain a separate Azure Artifacts cargo feed. Largely we just pull dependencies through from crates.io into that feed. Hence the config to replace the default cargo feed with our own. + +As a maintainer, if you need to update that feed, then you'll need to do the following: + +```cmd +az login +az account get-access-token --query "join(' ', ['Bearer', accessToken])" --output tsv | cargo login --registry Sudo_PublicPackages +``` + +That'll log you in via the Azure CLI and then log you into the cargo feed. That'll let you pull down the packages but oh yikes everything is awful. From d7a68badd8ff4c53dbb3e1db74cc00a10b23581b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 7 Aug 2024 13:38:48 -0500 Subject: [PATCH 2/4] replace cargo feed with new azdo one ; remove `which` dependency --- Cargo.lock | 72 +---------------------------------------- Cargo.toml | 1 - sudo/Cargo.toml | 8 +++-- sudo/build.rs | 19 +++++++++-- sudo/src/run_handler.rs | 21 ++++++++++-- 5 files changed, 43 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a669f0e..fbb87c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,12 +56,6 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" -[[package]] -name = "bitflags" -version = "2.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" - [[package]] name = "byteorder" version = "1.5.0" @@ -109,55 +103,18 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" -[[package]] -name = "either" -version = "1.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11157ac094ffbdde99aa67b23417ebdd801842852b500e395a45a9c0aac03e4a" - [[package]] name = "embed-manifest" version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "41cd446c890d6bed1d8b53acef5f240069ebef91d6fae7c5f52efe61fe8b5eae" -[[package]] -name = "errno" -version = "0.3.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245" -dependencies = [ - "libc", - "windows-sys", -] - -[[package]] -name = "home" -version = "0.5.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3d1354bf6b7235cb4a0576c2619fd4ed18183f689b12b006a0ee7329eeff9a5" -dependencies = [ - "windows-sys", -] - [[package]] name = "libc" version = "0.2.153" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" -[[package]] -name = "linux-raw-sys" -version = "0.4.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c" - -[[package]] -name = "once_cell" -version = "1.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" - [[package]] name = "proc-macro2" version = "1.0.78" @@ -176,19 +133,6 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "rustix" -version = "0.38.31" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ea3e1a662af26cd7a3ba09c0297a31af215563ecf42817c98df621387f4e949" -dependencies = [ - "bitflags 2.4.2", - "errno", - "libc", - "linux-raw-sys", - "windows-sys", -] - [[package]] name = "serde" version = "1.0.196" @@ -223,7 +167,6 @@ dependencies = [ "clap", "embed-manifest", "sudo_events", - "which", "win32resources", "windows", "windows-registry", @@ -299,19 +242,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "which" -version = "6.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fa5e0c10bf77f44aac573e498d1a82d5fbd5e91f6fc0a99e7be4b38e85e101c" -dependencies = [ - "either", - "home", - "once_cell", - "rustix", - "windows-sys", -] - [[package]] name = "widestring" version = "1.0.2" @@ -342,7 +272,7 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e50d0fa665033a19ecefd281b4fb5481eba2972dedbb5ec129c9392a206d652f" dependencies = [ - "bitflags 1.3.2", + "bitflags", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 3a77dd2..54210be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,6 @@ cc = "1.0" # See: https://docs.rs/clap/latest/clap/_features/index.html clap = { version = "4.4.7", default-features = false, features = ["std"] } embed-manifest = "1.4" -which = "6.0" win_etw_provider = "0.1.8" win_etw_macros = "0.1.8" windows = "0.57" diff --git a/sudo/Cargo.toml b/sudo/Cargo.toml index 6d221b1..f426f15 100644 --- a/sudo/Cargo.toml +++ b/sudo/Cargo.toml @@ -15,12 +15,16 @@ name = "sudo" winres.workspace = true cc.workspace = true embed-manifest.workspace = true -which = { workspace = true } + +[build-dependencies.windows] +workspace = true +features = [ + "Win32_Storage_FileSystem", +] [dependencies] clap = { workspace = true, default-features = false, features = ["color", "help", "usage", "error-context"] } -which = { workspace = true } windows-registry = { workspace = true } sudo_events = { path = "../sudo_events" } diff --git a/sudo/build.rs b/sudo/build.rs index e80dfd9..05d6f6c 100644 --- a/sudo/build.rs +++ b/sudo/build.rs @@ -1,6 +1,7 @@ use embed_manifest::embed_manifest_file; use std::path::PathBuf; use std::process::Command; +use windows::{core::*, Win32::Storage::FileSystem::*}; use { std::{env, io}, winres::WindowsResource, @@ -32,6 +33,21 @@ fn get_sdk_path() -> Option { sdk_path } +fn search_path(filename: &str) -> Result { + let filename = &HSTRING::from(filename); + let len = unsafe { SearchPathW(None, filename, None, None, None) }; + + if len == 0 { + return Err(Error::from_win32()); + } + + let mut buffer = vec![0; len as usize]; + let len = unsafe { SearchPathW(None, filename, None, Some(&mut buffer), None) }; + buffer.truncate(len as usize); + + Ok(String::from_utf16(&buffer)?) +} + fn get_sdk_tool(sdk_path: &Option, tool_name: &str) -> String { // seems like, in a VS tools prompt, midl.exe is in the path so the above // doesn't include the path. kinda weird but okay? @@ -47,8 +63,7 @@ fn get_sdk_tool(sdk_path: &Option, tool_name: &str) -> String { // we can just get the absolute path to the exe using the windows // path search. - let tool_path = which::which(tool_name).expect("Failed to find tool in path"); - tool_path.to_str().unwrap().to_owned() + search_path(tool_name).expect("Failed to find tool in path") } }; tool_path diff --git a/sudo/src/run_handler.rs b/sudo/src/run_handler.rs index a2e956e..4b5f55d 100644 --- a/sudo/src/run_handler.rs +++ b/sudo/src/run_handler.rs @@ -191,6 +191,21 @@ pub fn run_target( do_request(req, copy_env, manually_requested_dir) } +fn search_path(filename: &str) -> Result { + let filename = &HSTRING::from(filename); + let len = unsafe { SearchPathW(None, filename, None, None, None) }; + + if len == 0 { + return Err(Error::from_win32()); + } + + let mut buffer = vec![0; len as usize]; + let len = unsafe { SearchPathW(None, filename, None, Some(&mut buffer), None) }; + buffer.truncate(len as usize); + + Ok(String::from_utf16(&buffer)?) +} + /// Constructs an ElevateRequest from the given arguments. We'll package up /// handles, we'll separate out the application and args, and we'll do some /// other work to make sure the request is ready to go. @@ -262,7 +277,7 @@ fn prepare_request( event_log_request(true, &req); // Does the application exist somewhere on the path? - let where_result = which::which(&req.application); + let where_result = search_path(&req.application); if let Ok(path) = where_result { // It's a real file that exists on the PATH. @@ -271,7 +286,9 @@ fn prepare_request( // ensures that the elevated sudo will execute the same thing that was // found here in the unelevated context. - req.application = absolute_path(&path)?.to_string_lossy().to_string(); + req.application = absolute_path(Path::new(&path))? + .to_string_lossy() + .to_string(); adjust_args_for_gui_exes(&mut req); } else { tracing::trace_command_not_found(&req.application); From 098bd5170b147d28a3beb52b543ff8c5bd936a33 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 9 Aug 2024 05:59:35 -0500 Subject: [PATCH 3/4] fix the search_path to use extensions too --- Building.md | 11 ++++-- sudo/src/run_handler.rs | 75 +++++++++++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/Building.md b/Building.md index 37d72ff..b80cdb5 100644 --- a/Building.md +++ b/Building.md @@ -76,13 +76,18 @@ Note, if you run that on the public toolchain, you'll most definitely run into ` ### Notes on updating the cargo feed -For internal reasons, we need to maintain a separate Azure Artifacts cargo feed. Largely we just pull dependencies through from crates.io into that feed. Hence the config to replace the default cargo feed with our own. +For internal reasons, we need to maintain a separate Azure Artifacts cargo feed. +Largely we just pull dependencies through from crates.io into that feed. Hence +the config to replace the default cargo feed with our own. -As a maintainer, if you need to update that feed, then you'll need to do the following: +As a maintainer, if you need to update that feed, You should check out the steps +under ["Cargo" on this +page](https://dev.azure.com/shine-oss/sudo/_artifacts/feed/Sudo_PublicPackages). +TL;DR, do the following: ```cmd az login az account get-access-token --query "join(' ', ['Bearer', accessToken])" --output tsv | cargo login --registry Sudo_PublicPackages ``` -That'll log you in via the Azure CLI and then log you into the cargo feed. That'll let you pull down the packages but oh yikes everything is awful. +That'll log you in via the Azure CLI and then log you into the cargo feed. I believe that should allow you to pull packages through to the Azure feed, from the public feed. If it doesn't, maintainers should have the necessary permissions to add packages to the feed. diff --git a/sudo/src/run_handler.rs b/sudo/src/run_handler.rs index 4b5f55d..12f5455 100644 --- a/sudo/src/run_handler.rs +++ b/sudo/src/run_handler.rs @@ -191,19 +191,58 @@ pub fn run_target( do_request(req, copy_env, manually_requested_dir) } -fn search_path(filename: &str) -> Result { - let filename = &HSTRING::from(filename); - let len = unsafe { SearchPathW(None, filename, None, None, None) }; +/// Returns all the extensions from PATHEXT that are used for "executables" +fn get_path_extensions() -> Vec { + env::var("PATHEXT") + .unwrap_or_else(|_| { + // If PATHEXT isn't set, use the default + ".COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC".to_string() + }) + .split(';') + .map(|ext| ext.to_string()) + .collect() +} - if len == 0 { - return Err(Error::from_win32()); - } +/// Searches the PATH for the given filename, with the extensions from PATHEXT. +/// Returns the full path to the file if it's found, or an error if it's not. +fn search_path_with_extensions(filename: &str) -> Result { + let filename = HSTRING::from(filename); + let mut buffer = vec![0; 260]; + let extensions = get_path_extensions(); + for extension in extensions { + let extension_hstring = HSTRING::from(extension); + // SearchPathW will ignore the extension if the filename already has one. + let len: u32 = unsafe { + SearchPathW( + None, + &filename, + &extension_hstring, + Some(&mut buffer), + None, + ) + }; + + // If the function fails, the return value is zero. + if len == 0 { + continue; + } - let mut buffer = vec![0; len as usize]; - let len = unsafe { SearchPathW(None, filename, None, Some(&mut buffer), None) }; - buffer.truncate(len as usize); + // If the function succeeds, the value returned is the + // length of the string that is copied to the buffer, + // not including the terminating null character. + if len < buffer.len() as u32 { + buffer.truncate(len as usize); + return Ok(String::from_utf16(&buffer)?); + } - Ok(String::from_utf16(&buffer)?) + // If the return value is greater than nBufferLength, the value + // returned is the size of the buffer that is required to hold + // the path, including the terminating null character. + // + // Includes some padding just in case and because it's cheap. + buffer.resize((len + 64) as usize, 0); + } + Err(Error::from_win32()) } /// Constructs an ElevateRequest from the given arguments. We'll package up @@ -277,7 +316,7 @@ fn prepare_request( event_log_request(true, &req); // Does the application exist somewhere on the path? - let where_result = search_path(&req.application); + let where_result = search_path_with_extensions(&req.application); if let Ok(path) = where_result { // It's a real file that exists on the PATH. @@ -538,6 +577,20 @@ fn runas_admin_impl(exe: &OsStr, args: &OsStr, show: SHOW_WINDOW_CMD) -> Result< mod tests { use super::*; + #[test] + fn test_search_path_cmd() { + let path = search_path_with_extensions("cmd").unwrap(); + assert!(path.eq_ignore_ascii_case("C:\\Windows\\System32\\cmd.exe")); + let path = search_path_with_extensions("cmd.exe").unwrap(); + assert!(path.eq_ignore_ascii_case("C:\\Windows\\System32\\cmd.exe")); + } + #[test] + fn test_search_path_bad() { + // There's no way this file exists + let path = search_path_with_extensions("acaeb0cf7e91430eb8958a64bee752a7").unwrap_err(); + assert_eq!(path.code().0, E_FILENOTFOUND.0); + } + #[test] fn test_cmd_is_cui() { let app_name = "cmd".to_string(); From 739cb2a2f1c52174d272bf606cc66353412aefec Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 9 Aug 2024 06:00:01 -0500 Subject: [PATCH 4/4] fmt & clippy& --- sudo/src/run_handler.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/sudo/src/run_handler.rs b/sudo/src/run_handler.rs index 12f5455..98252b2 100644 --- a/sudo/src/run_handler.rs +++ b/sudo/src/run_handler.rs @@ -212,15 +212,8 @@ fn search_path_with_extensions(filename: &str) -> Result { for extension in extensions { let extension_hstring = HSTRING::from(extension); // SearchPathW will ignore the extension if the filename already has one. - let len: u32 = unsafe { - SearchPathW( - None, - &filename, - &extension_hstring, - Some(&mut buffer), - None, - ) - }; + let len: u32 = + unsafe { SearchPathW(None, &filename, &extension_hstring, Some(&mut buffer), None) }; // If the function fails, the return value is zero. if len == 0 {