From 05f25155bd37bdcc5f99386ea18c42c5a30b2599 Mon Sep 17 00:00:00 2001 From: ospab Date: Fri, 7 Aug 2026 17:19:36 +0300 Subject: [PATCH] feat(gui): one UAC prompt per machine instead of one per connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enabling TUN raised a consent dialog every single time, because the GUI elevated the helper with ShellExecuteW("runas") on each connect. Registers a Scheduled Task with RunLevel=HighestAvailable the first time TUN is used — that registration is the one prompt — and triggers the task on every later connect. Running a task is not an elevation request, so Windows shows no dialog for it. If the task is missing or cannot be triggered, the code falls back to the original direct elevated launch, so this can only improve on the old behaviour, never break it. A task stores a FIXED command line, so the per-launch port and token cannot be arguments. The helper gained --args-file and the GUI writes them to %LOCALAPPDATA%\OSTP\helper-args.json immediately before triggering; the helper deletes it after reading. That path keeps the token inside the trust boundary it already had — the helper runs elevated but as the same user, and no other user can read it, which a shared location would not guarantee. Registered from an XML definition rather than /TR: the exe and args paths would otherwise need quoting inside an already-quoted /TR value, escaped again through ShellExecuteW, which breaks as soon as either contains a space — and both do by default (Program Files, usernames with spaces). Validated that the generated XML parses with a path containing both a space and an ampersand. XML also lets DisallowStartIfOnBatteries=false and ExecutionTimeLimit=PT0S be stated explicitly, without which a laptop would refuse to start the tunnel on battery and Windows would kill it after three days. Deliberately NOT a Windows service, contrary to what I suggested earlier. The helper is one-shot by design: it force-exits after teardown because WinTun's blocking receive otherwise keeps the adapter and its default route alive and breaks the next connect. A persistent LocalSystem service would mean restructuring that lifecycle for the same end-user result. The task also runs with the user's own token rather than LocalSystem, which is less privilege for the same outcome. A service is still the better answer if the tunnel should come up before login — that is the one thing this cannot do. --- ostp-gui/src-tauri/src/lib.rs | 232 ++++++++++++++++++++++++++++++++++ ostp-tun-helper/src/main.rs | 30 +++++ 2 files changed, 262 insertions(+) diff --git a/ostp-gui/src-tauri/src/lib.rs b/ostp-gui/src-tauri/src/lib.rs index 2ac604e..cb09bab 100644 --- a/ostp-gui/src-tauri/src/lib.rs +++ b/ostp-gui/src-tauri/src/lib.rs @@ -803,8 +803,186 @@ fn find_helper_exe() -> Option { None } +/// Name of the Scheduled Task that runs the helper elevated without a prompt. +#[cfg(target_os = "windows")] +const HELPER_TASK_NAME: &str = "OSTP TUN Helper"; + +/// Fixed path the GUI writes launch parameters to, and the task's command line +/// reads them from. +/// +/// A Scheduled Task stores a FIXED command line, so the per-launch port and +/// token cannot travel as arguments. The file lives under the user's own +/// LOCALAPPDATA: the helper runs elevated but as the SAME user, so this keeps +/// the token inside the trust boundary it already had — no other user can read +/// it, which would not be true of a shared location. +#[cfg(target_os = "windows")] +fn helper_args_file() -> PathBuf { + let base = std::env::var_os("LOCALAPPDATA") + .map(PathBuf::from) + .unwrap_or_else(std::env::temp_dir); + base.join("OSTP").join("helper-args.json") +} + +/// Minimal XML text escaping for the values interpolated into the task +/// definition. Paths and usernames are attacker-irrelevant here but can easily +/// contain `&`, which would otherwise produce invalid XML and a confusing +/// schtasks parse failure. +#[cfg(target_os = "windows")] +fn xml_escape(s: &str) -> String { + s.replace('&', "&") + .replace('<', "<") + .replace('>', ">") + .replace('"', """) + .replace('\'', "'") +} + +/// Whether the elevated-launch Scheduled Task already exists. +#[cfg(target_os = "windows")] +fn helper_task_exists() -> bool { + use std::process::Command; + Command::new("schtasks") + .args(["/Query", "/TN", HELPER_TASK_NAME]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +/// Register the Scheduled Task. This is the ONLY step that needs elevation, and +/// it happens once per machine; every later tunnel start reuses the task. +/// +/// RunLevel=HIGHEST makes the task run elevated, and because a task launch is +/// not an elevation request, Windows shows no consent dialog for it. +#[cfg(target_os = "windows")] +fn install_helper_task(exe: &std::path::Path) -> anyhow::Result<()> { + let args_file = helper_args_file(); + if let Some(dir) = args_file.parent() { + std::fs::create_dir_all(dir)?; + } + + // Register from an XML definition rather than /TR. The command line would + // otherwise need the exe path and the args path quoted INSIDE an already + // quoted /TR value, escaped again through ShellExecuteW — a notoriously + // brittle chain when either path contains a space, which both of these do + // by default (Program Files, and usernames with spaces). XML also lets the + // battery and time-limit settings below be stated explicitly. + let user = format!( + "{}\\{}", + std::env::var("USERDOMAIN").unwrap_or_else(|_| "%COMPUTERNAME%".into()), + std::env::var("USERNAME").unwrap_or_default() + ); + let xml = format!( + r#" + + + Runs the OSTP TUN helper elevated so enabling the tunnel does not prompt for consent every time. + + + + {user} + InteractiveToken + HighestAvailable + + + + Parallel + false + false + false + false + PT0S + true + false + true + + + + {exe} + --args-file "{args}" + + + +"#, + user = xml_escape(&user), + exe = xml_escape(&exe.display().to_string()), + args = xml_escape(&args_file.display().to_string()), + ); + + // schtasks /Create /XML expects UTF-16LE with a BOM. + let xml_path = std::env::temp_dir().join(format!("ostp_task_{}.xml", rand::random::())); + let mut utf16: Vec = vec![0xFF, 0xFE]; + for unit in xml.encode_utf16() { + utf16.extend_from_slice(&unit.to_le_bytes()); + } + std::fs::write(&xml_path, &utf16)?; + + // Registering a HighestAvailable task is itself privileged: this is the one + // prompt, and it happens once per machine. + let schtasks = std::path::PathBuf::from("schtasks.exe"); + let params = format!( + "/Create /TN \"{}\" /XML \"{}\" /F", + HELPER_TASK_NAME, + xml_path.display() + ); + let result = shell_execute_elevated(&schtasks, ¶ms); + // Best-effort cleanup; schtasks may still be reading it, so ignore errors. + let _ = std::fs::remove_file(&xml_path); + result?; + + // schtasks runs asynchronously through ShellExecute; wait briefly for the + // task to appear rather than reporting success before it exists. + for _ in 0..20 { + if helper_task_exists() { + return Ok(()); + } + std::thread::sleep(std::time::Duration::from_millis(250)); + } + anyhow::bail!("the scheduled task did not appear after the elevation prompt (it may have been declined)") +} + #[cfg(target_os = "windows")] fn launch_as_admin(exe: &std::path::PathBuf, token: &str, port: u16) -> anyhow::Result<()> { + // Preferred path: hand the parameters over in a file and trigger the + // pre-registered task, which runs elevated with no prompt. Falls back to a + // direct elevated launch when the task is absent (first ever run, or the + // user removed it) — and that first run is also where the task gets created, + // so the prompt appears once rather than on every connect. + let args_file = helper_args_file(); + if let Some(dir) = args_file.parent() { + let _ = std::fs::create_dir_all(dir); + } + let payload = serde_json::json!({ "port": port, "token": token }); + let wrote_args = std::fs::write(&args_file, payload.to_string()).is_ok(); + + if wrote_args { + if !helper_task_exists() { + if let Err(e) = install_helper_task(exe) { + eprintln!("[OSTP] could not register the helper task ({e}); falling back to a direct elevated launch"); + } + } + if helper_task_exists() { + use std::process::Command; + let run = Command::new("schtasks") + .args(["/Run", "/TN", HELPER_TASK_NAME]) + .output(); + match run { + Ok(o) if o.status.success() => return Ok(()), + Ok(o) => eprintln!( + "[OSTP] schtasks /Run failed: {}", + String::from_utf8_lossy(&o.stderr).trim() + ), + Err(e) => eprintln!("[OSTP] schtasks /Run could not start: {e}"), + } + } + // Falling through: remove the file so a stale token is not left behind. + let _ = std::fs::remove_file(&args_file); + } + + launch_as_admin_direct(exe, token, port) +} + +/// The original one-prompt-per-launch path, kept as the fallback. +#[cfg(target_os = "windows")] +fn launch_as_admin_direct(exe: &std::path::PathBuf, token: &str, port: u16) -> anyhow::Result<()> { use std::ffi::OsStr; use std::os::windows::ffi::OsStrExt; use std::ptr::null_mut; @@ -857,6 +1035,60 @@ fn launch_as_admin(exe: &std::path::PathBuf, token: &str, port: u16) -> anyhow:: Ok(()) } +/// Run `exe` elevated with `params`, raising the UAC prompt. +/// +/// Shared by the fallback launch path and by the one-time task registration, so +/// both report a declined prompt the same way instead of ShellExecuteW's +/// pseudo-HINSTANCE being interpreted twice. +#[cfg(target_os = "windows")] +fn shell_execute_elevated(exe: &std::path::Path, params: &str) -> anyhow::Result<()> { + use std::ffi::OsStr; + use std::os::windows::ffi::OsStrExt; + use std::ptr::null_mut; + + let exe_wstr: Vec = exe.as_os_str().encode_wide().chain(Some(0)).collect(); + let verb_wstr: Vec = OsStr::new("runas").encode_wide().chain(Some(0)).collect(); + let params_wstr: Vec = OsStr::new(params).encode_wide().chain(Some(0)).collect(); + + #[link(name = "shell32")] + extern "system" { + fn ShellExecuteW(h: *mut std::ffi::c_void, op: *const u16, f: *const u16, p: *const u16, d: *const u16, s: i32) -> isize; + } + #[link(name = "kernel32")] + extern "system" { + fn GetLastError() -> u32; + } + + let cwd_path = std::env::current_exe().unwrap_or_else(|_| std::path::PathBuf::from(".")); + let dir_wstr: Vec = cwd_path + .parent() + .unwrap_or(std::path::Path::new(".")) + .as_os_str() + .encode_wide() + .chain(Some(0)) + .collect(); + + let ret = unsafe { + ShellExecuteW(null_mut(), verb_wstr.as_ptr(), exe_wstr.as_ptr(), params_wstr.as_ptr(), dir_wstr.as_ptr(), 1) + }; + + // 1223 is ERROR_CANCELLED, which lands in the ">32 means success" range — + // see the note in launch_as_admin_direct. + if ret == 1223 { + anyhow::bail!("UAC elevation was denied."); + } + if ret <= 32 { + let win_err = unsafe { GetLastError() }; + anyhow::bail!( + "Failed to request UAC elevation (ShellExecuteW ret={}, GetLastError={}, path={})", + ret, + win_err, + exe.display() + ); + } + Ok(()) +} + #[cfg(target_os = "linux")] fn launch_as_admin(exe: &PathBuf, token: &str, port: u16) -> Result<()> { use std::os::unix::fs::PermissionsExt; diff --git a/ostp-tun-helper/src/main.rs b/ostp-tun-helper/src/main.rs index d8ae30e..a8ebc18 100644 --- a/ostp-tun-helper/src/main.rs +++ b/ostp-tun-helper/src/main.rs @@ -24,6 +24,14 @@ fn log_to_file(msg: &str) { +/// Launch parameters handed over in a file rather than on the command line. +/// See the `--args-file` handling in `main` for why. +#[derive(Deserialize)] +struct HelperArgs { + port: u16, + token: String, +} + #[derive(Deserialize)] #[serde(tag = "cmd", rename_all = "lowercase")] enum GuiCmd { @@ -76,6 +84,28 @@ async fn main() -> Result<()> { let _ = std::fs::remove_file(path); // securely delete after reading } } + // Both port and token from one file. A Scheduled Task stores a FIXED + // command line, so anything that varies per launch cannot be passed as + // an argument — the GUI writes this file immediately before triggering + // the task instead. That indirection is what lets the task be created + // once (a single UAC prompt) and reused for every later connect without + // prompting again. + if args[i] == "--args-file" && i + 1 < args.len() { + let path = &args[i + 1]; + match std::fs::read_to_string(path) { + Ok(content) => { + let _ = std::fs::remove_file(path); // single use + match serde_json::from_str::(&content) { + Ok(parsed) => { + port = parsed.port; + expected_token = parsed.token; + } + Err(e) => log_to_file(&format!("Failed to parse --args-file: {e}")), + } + } + Err(e) => log_to_file(&format!("Failed to read --args-file {path}: {e}")), + } + } } log_to_file("Helper started (TCP mode)");