diff --git a/src/platform/windows/installer_handoff.rs b/src/platform/windows/installer_handoff.rs index 6e2380cf8..4b66e3ca5 100644 --- a/src/platform/windows/installer_handoff.rs +++ b/src/platform/windows/installer_handoff.rs @@ -1,12 +1,16 @@ +mod directory; + use super::{ installer_shell::{ get_system_executable, path_for_cmd_assignment, path_for_cmd_environment, run_elevated_and_wait, trusted_install_environment, BATCH_SHORTCUT_DECODE_FAILURE_EXIT_CODE, CMD_RELATIVE_PATH, + WIN7_SHELL_EXECUTE_MAX_PARAMETER_CHARS, }, validate_install_app_name, ResultType, }; use hbb_common::{ + anyhow::anyhow, bail, log, sha2::{Digest, Sha256}, }; @@ -99,10 +103,8 @@ fn verified_install_bootstrap( runner_directory: &Path, ) -> ResultType { let source = path_for_cmd_assignment(&script.path)?; - let runner = runner_directory.join(format!( - "rustdesk_install_{}.bat", - uuid::Uuid::new_v4().simple() - )); + let directory = path_for_cmd_assignment(runner_directory)?; + let runner = runner_directory.join("install.bat"); let runner = path_for_cmd_assignment(&runner)?; let cmd_path = get_system_executable(CMD_RELATIVE_PATH)?; let certutil_path = get_system_executable(CERTUTIL_RELATIVE_PATH)?; @@ -113,13 +115,13 @@ fn verified_install_bootstrap( // Short names preserve headroom under the Windows 7 ShellExecuteExW 2,048 // UTF-16-character parameter limit. Paths are stored with delayed expansion // disabled, then expanded indirectly so literal `!` survives: S=source, - // R=runner, Q=cmd.exe, H=certutil.exe, F=findstr.exe, C=created flag, E=exit code. + // R=runner, Q=cmd.exe, H=certutil.exe, F=findstr.exe, D=directory, E=exit code. Ok(format!( "setlocal DisableDelayedExpansion & set \"S={source}\" & set \"R={runner}\" & \ set \"Q={cmd}\" & set \"H={certutil}\" & set \"F={findstr}\" & \ - set \"C=0\" & set \"E=0\" & setlocal EnableDelayedExpansion & \ + set \"ERRORLEVEL=\" & set \"D={directory}\" & set \"E=0\" & setlocal EnableDelayedExpansion & \ if exist \"!R!\" (set \"E={INSTALL_HANDOFF_RUNNER_EXISTS_EXIT_CODE}\") else (\ - set \"C=1\" & copy /Y \"!S!\" \"!R!\" > nul || \ + copy /Y \"!S!\" \"!R!\" > nul || \ (set \"E={INSTALL_HANDOFF_COPY_FAILURE_EXIT_CODE}\") & \ if \"!E!\"==\"0\" (\"!H!\" -hashfile \"!R!\" SHA256 > \"!R!.hash\" || \ set \"E={INSTALL_HANDOFF_HASH_FAILURE_EXIT_CODE}\") & \ @@ -127,26 +129,51 @@ fn verified_install_bootstrap( set \"E={INSTALL_HANDOFF_HASH_MISMATCH_EXIT_CODE}\") & \ if \"!E!\"==\"0\" (\"!Q!\" /D /E:ON /V:OFF /C \"\"!R!\"\" & \ set \"E=!errorlevel!\")) & \ - if \"!C!\"==\"1\" (rd /s /q \"!R!.dir\" > nul 2>&1 & \ - del /f /q \"!R!\" \"!R!.*\" > nul 2>&1) & exit /b !E!", + rd /s /q \"!D!\" > nul 2>&1 & exit /b !E!", install_hash_pattern(&script.expected_hash), )) } -fn verified_install_parameters(script: &InstallCommandScript) -> ResultType { - let system_directory = get_system_executable("")?; - Ok(format!( +fn verified_install_parameters( + script: &InstallCommandScript, + directory: &Path, +) -> ResultType { + let parameters = format!( "/D /E:ON /V:ON /C {}", - verified_install_bootstrap(script, &system_directory)? - )) + verified_install_bootstrap(script, directory)? + ); + let length = parameters.encode_utf16().count(); + if length >= WIN7_SHELL_EXECUTE_MAX_PARAMETER_CHARS { + bail!("Elevated command is too long: {length} UTF-16 characters"); + } + Ok(parameters) } pub(super) fn run_cmds(cmds: String, show: bool, tip: &str) -> ResultType<()> { validate_install_app_name(&crate::get_app_name())?; let script = write_install_script(cmds)?; let cmd_path = get_system_executable(CMD_RELATIVE_PATH)?; - let parameters = verified_install_parameters(&script)?; - let exit_code = run_elevated_and_wait(&cmd_path, ¶meters, show)?; + let directory = directory::path()?; + let parameters = verified_install_parameters(&script, &directory)?; + directory::create(&directory)?; + let exit_code = run_elevated_and_wait(&cmd_path, ¶meters, show).map_err(|err| { + anyhow!( + "Installer handoff failed; protected directory may remain at {}: {err}", + directory.display() + ) + })?; + // Cleanup must not change the result of an installation that already ran. + match fs::symlink_metadata(&directory) { + Ok(_) => log::warn!( + "Protected installer directory may remain at {}", + directory.display() + ), + Err(err) if err.kind() == io::ErrorKind::NotFound => {} + Err(err) => log::warn!( + "Protected installer directory may remain at {}: {err}", + directory.display() + ), + } if exit_code != 0 { bail!( "{tip} failed with elevated exit code {exit_code}: {}", @@ -208,7 +235,7 @@ mod tests { .expect("install script should be created"); let bootstrap = verified_install_bootstrap(&script, &runner_dir) .expect("native verifier bootstrap should be generated"); - assert_native_handoff_structure(&script, &shortcut_commands, &bootstrap); + assert_native_handoff_structure(&script, &shortcut_commands, &runner_dir); let output = run_install_bootstrap_for_test(&bootstrap); assert!( @@ -218,15 +245,25 @@ mod tests { ); assert!(marker.exists(), "verified install script must execute"); std::fs::remove_file(&marker).expect("test marker should be removed"); + assert!( + !runner_dir.exists(), + "protected runner directory must be removed" + ); + std::fs::create_dir(&runner_dir).expect("runner directory should be created"); assert_replaced_install_script_is_rejected(&script, &runner_dir, &marker); - std::fs::remove_dir(runner_dir).expect("runner directory should be empty"); + assert!( + !runner_dir.exists(), + "failed handoff directory must be removed" + ); } fn assert_native_handoff_structure( script: &InstallCommandScript, shortcut_commands: &str, - bootstrap: &str, + runner_dir: &Path, ) { + let bootstrap = verified_install_bootstrap(script, runner_dir) + .expect("native verifier bootstrap should be generated"); assert!(shortcut_commands.contains("certutil")); assert!(shortcut_commands.contains("-decode")); assert!(!shortcut_commands.to_ascii_lowercase().contains("cscript")); @@ -240,8 +277,8 @@ mod tests { .collect::>() .join(" *"); assert!(bootstrap.contains(&format!("/R /I /X /C:\"{win7_hash_pattern}\""))); - let parameters = - verified_install_parameters(script).expect("elevated parameters should be generated"); + let parameters = verified_install_parameters(script, runner_dir) + .expect("elevated parameters should be generated"); assert!(bootstrap.contains("certutil.exe")); assert!(bootstrap.contains("findstr.exe")); assert!(!bootstrap.to_ascii_lowercase().contains("powershell")); diff --git a/src/platform/windows/installer_handoff/directory.rs b/src/platform/windows/installer_handoff/directory.rs new file mode 100644 index 000000000..aa2c152f8 --- /dev/null +++ b/src/platform/windows/installer_handoff/directory.rs @@ -0,0 +1,185 @@ +use super::super::{ + installer_shell::{get_system_executable, path_for_cmd_environment}, + wide_string, +}; +use hbb_common::{anyhow::anyhow, bail, ResultType}; +use std::{ + fs, mem, + os::windows::fs::MetadataExt, + path::{Path, PathBuf}, +}; +use windows::{ + core::{w, PCWSTR}, + Win32::{ + Foundation::{LocalFree, HLOCAL}, + Security::{ + Authorization::{ + ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + }, + PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, + }, + Storage::FileSystem::{CreateDirectoryW, FILE_ATTRIBUTE_REPARSE_POINT}, + }, +}; + +pub(super) fn path() -> ResultType { + let system = get_system_executable("")?; + let windows = system + .parent() + .ok_or_else(|| anyhow!("System directory has no parent"))?; + let temporary = windows.join("Temp"); + for parent in temporary.ancestors() { + let metadata = fs::symlink_metadata(parent).map_err(|err| { + anyhow!( + "Failed to inspect installer temporary parent {}: {err}", + parent.display() + ) + })?; + if !metadata.is_dir() || metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT.0 != 0 { + bail!( + "Installer temporary parent is not a regular directory: {}", + parent.display() + ); + } + } + Ok(temporary.join(format!( + "rustdesk-install-{}", + uuid::Uuid::new_v4().simple() + ))) +} + +pub(super) fn create(path: &Path) -> ResultType<()> { + let wide_path = wide_string(path_for_cmd_environment(path)?); + let mut descriptor = PSECURITY_DESCRIPTOR::default(); + // OWNER RIGHTS removes the ordinary creator's implicit WRITE_DAC permission. + // The owner can inspect the ACL, but only Administrators/SYSTEM can modify contents. + // https://learn.microsoft.com/windows-server/identity/ad-ds/manage/understand-special-identities-groups#owner-rights + unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + w!("D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;RC;;;OW)"), + SDDL_REVISION_1, + &mut descriptor, + None, + )?; + } + let attributes = SECURITY_ATTRIBUTES { + nLength: mem::size_of::() as u32, + lpSecurityDescriptor: descriptor.0, + bInheritHandle: false.into(), + }; + let result = unsafe { CreateDirectoryW(PCWSTR(wide_path.as_ptr()), Some(&attributes)) }; + unsafe { LocalFree(Some(HLOCAL(descriptor.0))) }; + result.map_err(|err| { + anyhow!( + "Failed to create protected installer directory {}: {err}", + path.display() + ) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use winapi::um::winnt::MAXIMUM_ALLOWED; + use windows::{ + core::{Owned, BOOL}, + Win32::{ + Foundation::HANDLE, + Security::{ + AccessCheck, + Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT}, + DuplicateToken, SecurityImpersonation, DACL_SECURITY_INFORMATION, GENERIC_MAPPING, + GROUP_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, PRIVILEGE_SET, + TOKEN_DUPLICATE, TOKEN_QUERY, + }, + Storage::FileSystem::{ + FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE, + READ_CONTROL, + }, + System::Threading::{GetCurrentProcess, OpenProcessToken}, + }, + }; + + #[test] + fn protected_directory_enforces_creator_permissions() { + let elevated = + super::super::super::is_elevated(None).expect("creator elevation should be available"); + // The user-owned parent allows cleanup through DELETE_CHILD. AccessCheck + // below checks the child's DACL, not that separate parent permission. + let directory = std::env::temp_dir().join(format!( + "rustdesk_installer_acl_{}", + uuid::Uuid::new_v4().simple() + )); + create(&directory).expect("production directory creation should succeed"); + let access = creator_directory_access(&directory); + fs::remove_dir(&directory).expect("test directory should be removed"); + let expected = if elevated { + FILE_ALL_ACCESS + } else { + READ_CONTROL + }; + assert_eq!( + access.expect("the created directory's access should be checked"), + (true, expected.0), + "only elevated administrators may modify the protected directory" + ); + } + + fn creator_directory_access(path: &Path) -> ResultType<(bool, u32)> { + let wide_path = wide_string(path_for_cmd_environment(path)?); + let mut descriptor = PSECURITY_DESCRIPTOR::default(); + unsafe { + GetNamedSecurityInfoW( + PCWSTR(wide_path.as_ptr()), + SE_FILE_OBJECT, + OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION, + None, + None, + None, + None, + &mut descriptor, + ) + .ok()?; + } + let _descriptor = unsafe { Owned::new(HLOCAL(descriptor.0)) }; + let token = creator_impersonation_token()?; + let mapping = GENERIC_MAPPING { + GenericRead: FILE_GENERIC_READ.0, + GenericWrite: FILE_GENERIC_WRITE.0, + GenericExecute: FILE_GENERIC_EXECUTE.0, + GenericAll: FILE_ALL_ACCESS.0, + }; + let mut privileges = PRIVILEGE_SET::default(); + let mut privilege_size = mem::size_of::() as u32; + let mut granted = 0; + let mut allowed = BOOL::default(); + unsafe { + AccessCheck( + descriptor, + *token, + MAXIMUM_ALLOWED, + &mapping, + Some(&mut privileges), + &mut privilege_size, + &mut granted, + &mut allowed, + )?; + } + Ok((allowed.as_bool(), granted)) + } + + fn creator_impersonation_token() -> ResultType> { + let mut token = HANDLE::default(); + unsafe { + OpenProcessToken( + GetCurrentProcess(), + TOKEN_QUERY | TOKEN_DUPLICATE, + &mut token, + )? + }; + let token = unsafe { Owned::new(token) }; + let mut impersonation = HANDLE::default(); + unsafe { DuplicateToken(*token, SecurityImpersonation, &mut impersonation)? }; + Ok(unsafe { Owned::new(impersonation) }) + } +}