From 53951fa56789060c42438a5e5378460ded0ffa65 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 20 Aug 2024 09:10:31 +0300 Subject: [PATCH 01/10] Devolutions host bootstrap --- Cargo.lock | 20 ++ Cargo.toml | 2 +- crates/win-api-wrappers/Cargo.toml | 6 +- crates/win-api-wrappers/src/lib.rs | 4 + crates/win-api-wrappers/src/process.rs | 270 ++++++++++++++- .../src/security/privilege.rs | 63 +++- crates/win-api-wrappers/src/session.rs | 27 ++ crates/win-api-wrappers/src/token.rs | 20 ++ devolutions-agent/Cargo.toml | 2 + devolutions-agent/src/config.rs | 21 ++ devolutions-agent/src/lib.rs | 6 + devolutions-agent/src/main.rs | 38 +- devolutions-agent/src/service.rs | 59 +++- devolutions-agent/src/session_manager/mod.rs | 325 ++++++++++++++++++ devolutions-gateway/src/tls.rs | 3 +- devolutions-host/Cargo.toml | 38 ++ devolutions-host/build.rs | 84 +++++ devolutions-host/src/config.rs | 260 ++++++++++++++ devolutions-host/src/dvc.rs | 147 ++++++++ devolutions-host/src/lib.rs | 14 + devolutions-host/src/log.rs | 21 ++ devolutions-host/src/main.rs | 46 +++ jetsocat/src/pipe.rs | 3 +- jetsocat/src/utils.rs | 3 +- 24 files changed, 1444 insertions(+), 38 deletions(-) create mode 100644 crates/win-api-wrappers/src/session.rs create mode 100644 devolutions-agent/src/session_manager/mod.rs create mode 100644 devolutions-host/Cargo.toml create mode 100644 devolutions-host/build.rs create mode 100644 devolutions-host/src/config.rs create mode 100644 devolutions-host/src/dvc.rs create mode 100644 devolutions-host/src/lib.rs create mode 100644 devolutions-host/src/log.rs create mode 100644 devolutions-host/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 1aa2e52c0..6ee791a57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1014,6 +1014,7 @@ dependencies = [ "tokio-rustls 0.26.0", "tracing", "uuid", + "win-api-wrappers", "windows 0.58.0", "winreg 0.52.0", ] @@ -1117,6 +1118,24 @@ dependencies = [ "tokio 1.38.1", ] +[[package]] +name = "devolutions-host" +version = "2024.3.1" +dependencies = [ + "anyhow", + "camino", + "cfg-if", + "ctrlc", + "devolutions-log", + "embed-resource", + "parking_lot", + "serde", + "serde_json", + "tap", + "tracing", + "windows 0.58.0", +] + [[package]] name = "devolutions-log" version = "0.0.0" @@ -5797,6 +5816,7 @@ dependencies = [ "anyhow", "base16ct", "thiserror", + "tracing", "windows 0.58.0", ] diff --git a/Cargo.toml b/Cargo.toml index 5c378cebc..91f37a355 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = [ "crates/*", "devolutions-agent", - "devolutions-gateway", + "devolutions-gateway", "devolutions-host", "jetsocat", "tools/generate-openapi", ] diff --git a/crates/win-api-wrappers/Cargo.toml b/crates/win-api-wrappers/Cargo.toml index 03601d947..f00d90951 100644 --- a/crates/win-api-wrappers/Cargo.toml +++ b/crates/win-api-wrappers/Cargo.toml @@ -11,19 +11,20 @@ publish = false base16ct = { version = "0.2.0", features = ["alloc"] } anyhow = "1.0.86" thiserror = "1.0.63" +tracing = "0.1" [dependencies.windows] version = "0.58.0" features = [ "Win32_Foundation", "Win32_NetworkManagement_NetManagement", - "Win32_Security", "Win32_Security_Authentication_Identity", "Win32_Security_Authorization", - "Win32_Security_Cryptography", "Win32_Security_Cryptography_Catalog", "Win32_Security_Cryptography_Sip", + "Win32_Security_Cryptography", "Win32_Security_WinTrust", + "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Com", "Win32_System_Diagnostics_Debug", @@ -38,6 +39,7 @@ features = [ "Win32_System_Pipes", "Win32_System_ProcessStatus", "Win32_System_Registry", + "Win32_System_RemoteDesktop", "Win32_System_Rpc", "Win32_System_StationsAndDesktops", "Win32_System_SystemServices", diff --git a/crates/win-api-wrappers/src/lib.rs b/crates/win-api-wrappers/src/lib.rs index c200e342e..a0e44bbe5 100644 --- a/crates/win-api-wrappers/src/lib.rs +++ b/crates/win-api-wrappers/src/lib.rs @@ -1,3 +1,6 @@ +#[macro_use] +extern crate tracing; + #[cfg(target_os = "windows")] #[path = ""] mod lib_win { @@ -8,6 +11,7 @@ mod lib_win { pub mod identity; pub mod process; pub mod security; + pub mod session; pub mod thread; pub mod token; pub mod utils; diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 0f0da89a0..10aca9cf1 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -6,7 +6,7 @@ use std::os::windows::ffi::OsStringExt; use std::path::{Path, PathBuf}; use std::{ptr, slice}; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use crate::handle::{Handle, HandleWrapper}; use crate::security::acl::{RawSecurityAttributes, SecurityAttributes}; @@ -19,7 +19,10 @@ use windows::core::PCWSTR; use windows::Win32::Foundation::{ FreeLibrary, ERROR_INCORRECT_SIZE, HANDLE, HMODULE, MAX_PATH, WAIT_EVENT, WAIT_FAILED, }; -use windows::Win32::Security::TOKEN_ACCESS_MASK; +use windows::Win32::Security::{ + SE_ASSIGNPRIMARYTOKEN_NAME, SE_INCREASE_QUOTA_NAME, SE_TCB_NAME, TOKEN_ACCESS_MASK, TOKEN_ADJUST_PRIVILEGES, + TOKEN_QUERY, +}; use windows::Win32::System::Com::{COINIT_APARTMENTTHREADED, COINIT_DISABLE_OLE1DDE}; use windows::Win32::System::Diagnostics::Debug::{ReadProcessMemory, WriteProcessMemory}; use windows::Win32::System::LibraryLoader::{ @@ -27,15 +30,21 @@ use windows::Win32::System::LibraryLoader::{ }; use windows::Win32::System::Threading::{ CreateProcessAsUserW, CreateRemoteThread, GetCurrentProcess, GetExitCodeProcess, OpenProcess, OpenProcessToken, - QueryFullProcessImageNameW, WaitForSingleObject, CREATE_UNICODE_ENVIRONMENT, EXTENDED_STARTUPINFO_PRESENT, - INFINITE, LPPROC_THREAD_ATTRIBUTE_LIST, LPTHREAD_START_ROUTINE, PEB, PROCESS_ACCESS_RIGHTS, - PROCESS_BASIC_INFORMATION, PROCESS_CREATION_FLAGS, PROCESS_INFORMATION, PROCESS_NAME_WIN32, STARTUPINFOEXW, - STARTUPINFOW, STARTUPINFOW_FLAGS, + QueryFullProcessImageNameW, TerminateProcess, WaitForSingleObject, CREATE_UNICODE_ENVIRONMENT, + EXTENDED_STARTUPINFO_PRESENT, INFINITE, LPPROC_THREAD_ATTRIBUTE_LIST, LPTHREAD_START_ROUTINE, PEB, + PROCESS_ACCESS_RIGHTS, PROCESS_BASIC_INFORMATION, PROCESS_CREATION_FLAGS, PROCESS_INFORMATION, PROCESS_NAME_WIN32, + PROCESS_TERMINATE, STARTUPINFOEXW, STARTUPINFOW, STARTUPINFOW_FLAGS, }; use windows::Win32::UI::Shell::{ShellExecuteExW, SEE_MASK_NOCLOSEPROCESS, SHELLEXECUTEINFOW}; use windows::Win32::UI::WindowsAndMessaging::SHOW_WINDOW_CMD; +use super::security::privilege::ScopedPrivileges; use super::utils::{size_of_u32, ComContext}; +use windows::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, Process32FirstW, Process32NextW, PROCESSENTRY32W, TH32CS_SNAPPROCESS, +}; +use windows::Win32::System::Environment::{CreateEnvironmentBlock, DestroyEnvironmentBlock}; +use windows::Win32::System::RemoteDesktop::ProcessIdToSessionId; #[derive(Debug)] pub struct Process { @@ -549,6 +558,114 @@ pub struct ProcessInformation { pub thread_id: u32, } +pub struct ProcessEntry32Iterator { + snapshot_handle: Handle, + process_entry: PROCESSENTRY32W, + first: bool, +} + +impl ProcessEntry32Iterator { + pub fn new() -> Result { + // SAFETY: `CreateToolhelp32Snapshot` call is always safe and `Owned` wrapper ensures + // that the handle is properly closed when the iterator is dropped. + let snapshot_handle = unsafe { Handle::new(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0)?, true) }; + + // SAFETY: It is safe to zero out the structure as it is a simple POD type. + let mut process_entry: PROCESSENTRY32W = unsafe { mem::zeroed() }; + process_entry.dwSize = mem::size_of::().try_into().unwrap(); + + Ok(ProcessEntry32Iterator { + snapshot_handle, + process_entry, + first: true, + }) + } +} + +impl Iterator for ProcessEntry32Iterator { + type Item = ProcessEntry; + + fn next(&mut self) -> Option { + let result = if self.first { + // SAFETY: `windows` library ensures that `snapshot` handle is correct on creation, + // therefore it is safe to call Process32First + unsafe { Process32FirstW(self.snapshot_handle.raw(), &mut self.process_entry as *mut _) } + } else { + // SAFETY: `Process32Next` is safe to call because the `snapshot_handle` is valid while it + // is owned by the iterator. + unsafe { Process32NextW(self.snapshot_handle.raw(), &mut self.process_entry as *mut _) } + }; + + match result { + Err(err) if err.code() == ERROR_NO_MORE_FILES.to_hresult() => None, + Err(err) => { + error!(%err, "Failed to iterate over processes"); + None + } + Ok(()) => { + self.first = false; + Some(ProcessEntry(self.process_entry)) + } + } + } +} + +pub struct ProcessEntry(PROCESSENTRY32W); + +impl ProcessEntry { + pub fn process_id(&self) -> u32 { + self.0.th32ProcessID + } + + pub fn executable_name(&self) -> Result { + // NOTE: If for some reason szExeFile all 260 bytes filled and there is no null terminator, + // then the executable name will be truncated. + let exe_name_length = self + .0 + .szExeFile + .iter() + .position(|&c| c == 0) + .ok_or(anyhow!("Executable name null terminator not found"))?; + + let name = String::from_utf16(&self.0.szExeFile[..exe_name_length]) + .map_err(|_| anyhow!("Invalid executable name UTF16 encoding"))?; + + Ok(name) + } +} + +enum ProcessEnvironment { + OsDefined(*const c_void), + Custom(Vec), +} + +impl ProcessEnvironment { + fn as_mut_ptr(&self) -> Option<*const c_void> { + match self { + ProcessEnvironment::OsDefined(ptr) => Some(*ptr), + ProcessEnvironment::Custom(vec) => Some(vec.as_ptr() as *const _), + } + } +} + +impl Drop for ProcessEnvironment { + fn drop(&mut self) { + match self { + ProcessEnvironment::OsDefined(block) => { + // SAFETY: `block` is checked to be valid before free. + unsafe { + if !block.is_null() { + if let Err(err) = DestroyEnvironmentBlock(*block) { + warn!(%err, "Failed to destroy environment block"); + } + } + }; + } + _ => {} + } + } +} + // Goal is to wrap `CreateProcessAsUserW`, which has a lot of arguments. #[allow(clippy::too_many_arguments)] pub fn create_process_as_user( @@ -566,7 +683,17 @@ pub fn create_process_as_user( let application_name = application_name.map(WideString::from).unwrap_or_default(); let current_directory = current_directory.map(WideString::from).unwrap_or_default(); - let environment = environment.map(serialize_environment).transpose()?; + let environment = if let Some(env) = environment { + ProcessEnvironment::Custom(serialize_environment(env)?) + } else { + let mut environment: *mut c_void = ptr::null_mut(); + + if let Some(token) = token { + unsafe { CreateEnvironmentBlock(&mut environment, token.handle().raw(), false) }?; + } + + ProcessEnvironment::OsDefined(environment as *const _) + }; let mut command_line = command_line .map(CommandLine::to_command_line) @@ -593,7 +720,7 @@ pub fn create_process_as_user( thread_attributes.as_ref().map(|x| x.as_raw() as *const _), inherit_handles, creation_flags, - environment.as_ref().map(|x| x.as_ptr().cast()), + environment.as_mut_ptr(), current_directory.as_pcwstr(), &startup_info.as_raw()?.StartupInfo, &mut raw_process_information, @@ -613,3 +740,130 @@ pub fn create_process_as_user( thread_id: raw_process_information.dwThreadId, }) } + +/// Starts new process in the specified session. Note that this function requires the current +/// process to have `SYSTEM`-level permissions. Use with caution. +pub fn create_process_in_session( + session_id: u32, + application_name: Option<&Path>, + command_line: Option<&CommandLine>, + process_attributes: Option<&SecurityAttributes>, + thread_attributes: Option<&SecurityAttributes>, + inherit_handles: bool, + creation_flags: PROCESS_CREATION_FLAGS, + environment: Option<&HashMap>, + current_directory: Option<&Path>, + startup_info: &mut StartupInfo, +) -> Result { + let mut current_process_token = Process::current_process().token(TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY)?; + + // (needs investigation) Setting all of these at once fails and crashes the process. + // In `wayk-agent` project they are set one by one. + let mut _priv_tcb = ScopedPrivileges::enter(&mut current_process_token, &[SE_TCB_NAME])?; + let mut _priv_primary = ScopedPrivileges::enter(_priv_tcb.token_mut(), &[SE_ASSIGNPRIMARYTOKEN_NAME])?; + let _priv_quota = ScopedPrivileges::enter(_priv_primary.token_mut(), &[SE_INCREASE_QUOTA_NAME])?; + + let mut session_token = Token::for_session(session_id)?; + + session_token.set_session_id(session_id)?; + session_token.set_ui_access(1)?; + + create_process_as_user( + Some(&session_token), + application_name, + command_line, + process_attributes, + thread_attributes, + inherit_handles, + creation_flags, + environment, + current_directory, + startup_info, + ) +} + +pub fn is_process_running(process_name: &str) -> Result { + is_process_running_impl(process_name, None) +} + +pub fn is_process_running_in_session(process_name: &str, session_id: u32) -> Result { + is_process_running_impl(process_name, Some(session_id)) +} + +fn is_process_running_impl(process_name: &str, session_id: Option) -> Result { + for process in ProcessEntry32Iterator::new()? { + if let Some(session_id) = session_id { + let actual_session = match process_id_to_session(process.process_id()) { + Ok(session) => session, + Err(_) => { + continue; + } + }; + + if session_id != actual_session { + continue; + } + } + + if str::eq_ignore_ascii_case(process.executable_name()?.as_str(), process_name) { + return Ok(true); + } + } + + Ok(false) +} + +pub fn terminate_process_by_name(process_name: &str) -> Result { + terminate_process_by_name_impl(process_name, None) +} + +pub fn terminate_process_by_name_in_session(process_name: &str, session_id: u32) -> Result { + terminate_process_by_name_impl(process_name, Some(session_id)) +} + +fn terminate_process_by_name_impl(process_name: &str, session_id: Option) -> Result { + for process in ProcessEntry32Iterator::new()? { + if let Some(session_id) = session_id { + let actual_session = match process_id_to_session(process.process_id()) { + Ok(session) => session, + Err(_) => { + continue; + } + }; + + if session_id != actual_session { + continue; + } + } + + if str::eq_ignore_ascii_case(process.executable_name()?.as_str(), process_name) { + let process = unsafe { OpenProcess(PROCESS_TERMINATE, false, process.process_id()) }; + + match process { + Ok(process) => { + unsafe { + if let Err(err) = TerminateProcess(process, 1) { + warn!(process_name, session_id, %err, "TerminateProcess failed"); + return Ok(false); + } + } + + return Ok(true); + } + Err(err) => { + warn!(process_name, session_id, %err, "OpenProcess failed"); + continue; + } + } + } + } + + Ok(false) +} + +fn process_id_to_session(pid: u32) -> Result { + let mut session_id = 0; + // SAFETY: `session_id` is always pointing to a valid memory location. + unsafe { ProcessIdToSessionId(pid, &mut session_id as *mut _) }?; + Ok(session_id) +} diff --git a/crates/win-api-wrappers/src/security/privilege.rs b/crates/win-api-wrappers/src/security/privilege.rs index 497e608e5..e70ba6ef3 100644 --- a/crates/win-api-wrappers/src/security/privilege.rs +++ b/crates/win-api-wrappers/src/security/privilege.rs @@ -4,7 +4,7 @@ use std::sync::OnceLock; use anyhow::Result; use crate::process::Process; -use crate::token::Token; +use crate::token::{Token, TokenPrivilegesAdjustment}; use crate::utils::{slice_from_ptr, Snapshot, WideString}; use windows::core::PCWSTR; use windows::Win32::Foundation::LUID; @@ -104,6 +104,67 @@ pub fn find_token_with_privilege(privilege: LUID) -> Result> { })) } +/// ScopedPrivilege enables a Windows privilege for the lifetime of the object and +/// disables it when going out of scope. +/// +/// Token is borrowed to ensure that the token is alive throughout the lifetime of the scope. +pub struct ScopedPrivileges<'a> { + token: &'a mut Token, + token_privileges: Vec, + description: String, +} + +impl<'a> ScopedPrivileges<'a> { + pub fn enter(token: &'a mut Token, privileges: &[PCWSTR]) -> Result> { + let mut token_privileges = Vec::with_capacity(privileges.len()); + + for privilege in privileges.iter().copied() { + let luid = lookup_privilege_value(None, privilege)?; + token_privileges.push(luid); + } + + let description = privileges + .iter() + .map(|p| { + // SAFETY: `p` is a valid pointer to a NUL-terminated string. + String::from_utf16_lossy(unsafe { p.as_wide() }) + }) + .reduce(|mut acc, value| { + acc.push_str(", "); + acc.push_str(&value); + acc + }) + .unwrap_or_default(); + + token.adjust_privileges(&TokenPrivilegesAdjustment::Enable(token_privileges.clone()))?; + + Ok(ScopedPrivileges { + token, + token_privileges, + description, + }) + } + + pub fn token(&self) -> &Token { + &self.token + } + + pub fn token_mut(&mut self) -> &mut Token { + &mut self.token + } +} + +impl Drop for ScopedPrivileges<'_> { + fn drop(&mut self) { + if let Err(err) = self + .token + .adjust_privileges(&TokenPrivilegesAdjustment::Disable(self.token_privileges.clone())) + { + error!(%err, "Failed to disable ScopedPrivileges({})", self.description); + } + } +} + #[rustfmt::skip] pub fn default_admin_privileges() -> &'static TokenPrivileges { static PRIVS: OnceLock = OnceLock::new(); diff --git a/crates/win-api-wrappers/src/session.rs b/crates/win-api-wrappers/src/session.rs new file mode 100644 index 000000000..31a96e4cb --- /dev/null +++ b/crates/win-api-wrappers/src/session.rs @@ -0,0 +1,27 @@ +use super::handle::Handle; +use super::process::Process; +use super::security::privilege::ScopedPrivileges; +use anyhow::Result; +use windows::Win32::Foundation::{ERROR_NO_TOKEN, HANDLE}; +use windows::Win32::Security::{SE_TCB_NAME, TOKEN_ADJUST_PRIVILEGES, TOKEN_QUERY}; +use windows::Win32::System::RemoteDesktop::WTSQueryUserToken; + +/// Returns true if a user is logged in the provided session. +pub fn session_has_logged_in_user(session_id: u32) -> Result { + let mut current_process_token = Process::current_process().token(TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY)?; + + let mut _priv_tcb = ScopedPrivileges::enter(&mut current_process_token, &[SE_TCB_NAME])?; + + let mut handle = HANDLE::default(); + + // SAFETY: `WTSQueryUserToken` is safe to call with a valid session id and handle memory ptr. + match unsafe { WTSQueryUserToken(session_id, &mut handle as *mut _) } { + Err(err) if err.code() == ERROR_NO_TOKEN.to_hresult() => Ok(false), + Err(err) => Err(err.into()), + Ok(()) => { + // Close handle immediately. + let _handle: Handle = Handle::new(handle, true); + Ok(true) + } + } +} diff --git a/crates/win-api-wrappers/src/token.rs b/crates/win-api-wrappers/src/token.rs index 987603203..ecc44c512 100644 --- a/crates/win-api-wrappers/src/token.rs +++ b/crates/win-api-wrappers/src/token.rs @@ -45,6 +45,7 @@ use windows::Win32::Security::{ use windows::Win32::System::SystemServices::SE_GROUP_LOGON_ID; use super::utils::size_of_u32; +use windows::Win32::System::RemoteDesktop::WTSQueryUserToken; #[derive(Debug)] pub struct Token { @@ -64,6 +65,17 @@ impl Token { } } + pub fn for_session(session_id: u32) -> Result { + let mut user_token = HANDLE::default(); + + // SAFETY: query user token is always safe if dst pointer is valid. + unsafe { WTSQueryUserToken(session_id, &mut user_token as *mut _)? }; + + Ok(Self { + handle: user_token.into(), + }) + } + // Wrapper around `NtCreateToken`, which has a lot of arguments. #[allow(clippy::too_many_arguments)] pub fn create_token( @@ -339,6 +351,14 @@ impl Token { self.set_information_raw(windows::Win32::Security::TokenSessionId, &session_id) } + pub fn ui_access(&self) -> Result { + self.information_raw::(windows::Win32::Security::TokenUIAccess) + } + + pub fn set_ui_access(&mut self, ui_access: u32) -> Result<()> { + self.set_information_raw(windows::Win32::Security::TokenUIAccess, &ui_access) + } + pub fn mandatory_policy(&self) -> Result { Ok(self .information_raw::(windows::Win32::Security::TokenMandatoryPolicy)? diff --git a/devolutions-agent/Cargo.toml b/devolutions-agent/Cargo.toml index 5b5a8952a..e75c72845 100644 --- a/devolutions-agent/Cargo.toml +++ b/devolutions-agent/Cargo.toml @@ -63,6 +63,7 @@ thiserror = "1" uuid = { version = "1.10", features = ["v4"] } winreg = "0.52" devolutions-pedm = { path = "../crates/devolutions-pedm" } +win-api-wrappers = { path = "../crates/win-api-wrappers" } [target.'cfg(windows)'.dependencies.windows] version = "0.58" @@ -74,6 +75,7 @@ features = [ "Win32_Security_Cryptography", "Win32_Security_Authorization", "Win32_System_ApplicationInstallationAndServicing", + "Win32_System_RemoteDesktop", ] [target.'cfg(windows)'.build-dependencies] diff --git a/devolutions-agent/src/config.rs b/devolutions-agent/src/config.rs index 9dc009b83..7dd740b9d 100644 --- a/devolutions-agent/src/config.rs +++ b/devolutions-agent/src/config.rs @@ -18,6 +18,7 @@ pub struct Conf { pub updater: dto::UpdaterConf, pub remote_desktop: RemoteDesktopConf, pub pedm: dto::PedmConf, + pub session_host: dto::SessionHostConf, pub debug: dto::DebugConf, } @@ -44,6 +45,7 @@ impl Conf { updater: conf_file.updater.clone().unwrap_or_default(), remote_desktop, pedm: conf_file.pedm.clone().unwrap_or_default(), + session_host: conf_file.session_host.clone().unwrap_or_default(), debug: conf_file.debug.clone().unwrap_or_default(), }) } @@ -253,6 +255,19 @@ pub mod dto { } } + #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] + #[serde(rename_all = "PascalCase")] + pub struct SessionHostConf { + /// Enable Session host module (disabled by default) + pub enabled: bool, + } + + impl Default for SessionHostConf { + fn default() -> Self { + Self { enabled: false } + } + } + /// Source of truth for Agent configuration /// /// This struct represents the JSON file used for configuration as close as possible @@ -275,10 +290,15 @@ pub mod dto { #[serde(skip_serializing_if = "Option::is_none")] pub remote_desktop: Option, + /// Devolutions PEDM configuration #[serde(default, skip_serializing_if = "Option::is_none")] pub pedm: Option, + /// Devolutions Session Host configuration + #[serde(default, skip_serializing_if = "Option::is_none")] + pub session_host: Option, + /// (Unstable) Unsafe debug options for developers #[serde(rename = "__debug__", skip_serializing_if = "Option::is_none")] pub debug: Option, @@ -299,6 +319,7 @@ pub mod dto { remote_desktop: None, pedm: None, debug: None, + session_host: Some(SessionHostConf { enabled: true }), rest: serde_json::Map::new(), } } diff --git a/devolutions-agent/src/lib.rs b/devolutions-agent/src/lib.rs index f6c82ec87..4a2d32d22 100644 --- a/devolutions-agent/src/lib.rs +++ b/devolutions-agent/src/lib.rs @@ -9,5 +9,11 @@ mod log; mod remote_desktop; pub mod service; +#[cfg(windows)] +mod session_manager; + #[cfg(windows)] mod updater; + +pub enum CustomAgentServiceEvent {} +pub type AgentServiceEvent = ceviche::ServiceEvent; diff --git a/devolutions-agent/src/main.rs b/devolutions-agent/src/main.rs index 070946f85..1622ac50d 100644 --- a/devolutions-agent/src/main.rs +++ b/devolutions-agent/src/main.rs @@ -15,19 +15,18 @@ use std::env; use std::sync::mpsc; use ceviche::controller::*; -use ceviche::{Service, ServiceEvent}; +use ceviche::Service; use devolutions_agent::config::ConfHandle; use devolutions_agent::service::{AgentService, DESCRIPTION, DISPLAY_NAME, SERVICE_NAME}; +use devolutions_agent::AgentServiceEvent; const BAD_CONFIG_ERR_CODE: u32 = 1; const START_FAILED_ERR_CODE: u32 = 2; -enum AgentServiceEvent {} - fn agent_service_main( - rx: mpsc::Receiver>, - _tx: mpsc::Sender>, + rx: mpsc::Receiver, + _tx: mpsc::Sender, _args: Vec, _standalone_mode: bool, ) -> u32 { @@ -53,13 +52,34 @@ fn agent_service_main( } } + let mut service_event_tx = service.service_event_tx(); + loop { if let Ok(control_code) = rx.recv() { info!(%control_code, "Received control code"); - if let ServiceEvent::Stop = control_code { - service.stop(); - break; + match control_code { + AgentServiceEvent::Stop => { + break; + } + AgentServiceEvent::SessionConnect(_) + | AgentServiceEvent::SessionDisconnect(_) + | AgentServiceEvent::SessionRemoteConnect(_) + | AgentServiceEvent::SessionRemoteDisconnect(_) + | AgentServiceEvent::SessionLogon(_) + | AgentServiceEvent::SessionLogoff(_) => { + if let Some(tx) = service_event_tx.as_mut() { + match tx.send(control_code) { + Ok(()) => {} + Err(err) => { + error!(%err, "Failed to send event to session manager"); + service_event_tx = None; + } + } + } + } + + _ => {} } } } @@ -101,7 +121,7 @@ fn main() { let _tx = tx.clone(); ctrlc::set_handler(move || { - let _ = tx.send(ServiceEvent::Stop); + let _ = tx.send(AgentServiceEvent::Stop); }) .expect("failed to register Ctrl-C handler"); diff --git a/devolutions-agent/src/service.rs b/devolutions-agent/src/service.rs index 58ec5b312..a68f24f11 100644 --- a/devolutions-agent/src/service.rs +++ b/devolutions-agent/src/service.rs @@ -1,10 +1,14 @@ use tokio::runtime::{self, Runtime}; +use tokio::sync::mpsc; use crate::config::ConfHandle; use crate::log::AgentLog; use crate::remote_desktop::RemoteDesktopTask; #[cfg(windows)] +use crate::session_manager::SessionManager; +#[cfg(windows)] use crate::updater::UpdaterTask; +use crate::AgentServiceEvent; use anyhow::Context; use devolutions_gateway_task::{ChildTask, ShutdownHandle, ShutdownSignal}; use devolutions_log::{self, LogDeleterTask, LoggerGuard}; @@ -16,6 +20,13 @@ pub const SERVICE_NAME: &str = "devolutions-agent"; pub const DISPLAY_NAME: &str = "Devolutions Agent"; pub const DESCRIPTION: &str = "Devolutions Agent service"; +struct TasksCtx { + /// Spawned service tasks + tasks: Tasks, + /// Sender side of the service event channel (Used for session manager module) + service_event_tx: Option>, +} + #[allow(clippy::large_enum_variant)] // `Running` variant is bigger than `Stopped` but we don't care enum AgentState { Stopped, @@ -29,6 +40,7 @@ pub struct AgentService { conf_handle: ConfHandle, state: AgentState, _logger_guard: LoggerGuard, + service_event_tx: Option>, } impl AgentService { @@ -57,6 +69,7 @@ impl AgentService { Ok(AgentService { conf_handle, state: AgentState::Stopped, + service_event_tx: None, _logger_guard: logger_guard, }) } @@ -70,7 +83,10 @@ impl AgentService { let config = self.conf_handle.clone(); // create_futures needs to be run in the runtime in order to bind the sockets. - let tasks = runtime.block_on(spawn_tasks(config))?; + let TasksCtx { + tasks, + service_event_tx, + } = runtime.block_on(spawn_tasks(config))?; trace!("Tasks created"); @@ -94,6 +110,7 @@ impl AgentService { } }); + self.service_event_tx = service_event_tx; self.state = AgentState::Running { shutdown_handle: tasks.shutdown_handle, runtime, @@ -147,6 +164,10 @@ impl AgentService { } } } + + pub fn service_event_tx(&self) -> Option> { + self.service_event_tx.clone() + } } struct Tasks { @@ -175,7 +196,7 @@ impl Tasks { } } -async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { +async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { let conf = conf_handle.get_conf(); let mut tasks = Tasks::new(); @@ -183,18 +204,34 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { tasks.register(LogDeleterTask::::new(conf.log_file.clone())); #[cfg(windows)] - if conf.updater.enabled { - tasks.register(UpdaterTask::new(conf_handle.clone())); - } + let service_event_tx = { + if conf.updater.enabled { + tasks.register(UpdaterTask::new(conf_handle.clone())); + } + + if conf.pedm.enabled { + tasks.register(PedmTask::new()) + } + + if conf.session_host.enabled { + let session_manager = SessionManager::default(); + let tx = session_manager.service_event_tx(); + tasks.register(session_manager); + Some(tx) + } else { + None + } + }; + + #[cfg(not(windows))] + let service_event_tx = None; if conf.debug.enable_unstable && conf.remote_desktop.enabled { tasks.register(RemoteDesktopTask::new(conf_handle)); } - #[cfg(windows)] - if conf.pedm.enabled { - tasks.register(PedmTask::new()) - } - - Ok(tasks) + Ok(TasksCtx { + tasks, + service_event_tx, + }) } diff --git a/devolutions-agent/src/session_manager/mod.rs b/devolutions-agent/src/session_manager/mod.rs new file mode 100644 index 000000000..88d82577f --- /dev/null +++ b/devolutions-agent/src/session_manager/mod.rs @@ -0,0 +1,325 @@ +//! Module for starting and managing the Devolutions Host process in user sessions. + +use tokio::sync::{mpsc, RwLock}; + +use crate::AgentServiceEvent; +use async_trait::async_trait; +use ceviche::controller::Session; +use devolutions_gateway_task::{ShutdownSignal, Task}; +use std::collections::BTreeMap; +use std::fmt::Debug; + +use camino::Utf8PathBuf; +use win_api_wrappers::process::{ + create_process_in_session, is_process_running_in_session, terminate_process_by_name_in_session, StartupInfo, +}; +use win_api_wrappers::session::session_has_logged_in_user; +use win_api_wrappers::utils::{CommandLine, WideString}; +use windows::Win32::System::Threading::{ + CREATE_NEW_CONSOLE, CREATE_UNICODE_ENVIRONMENT, NORMAL_PRIORITY_CLASS, STARTF_USESHOWWINDOW, +}; +use windows::Win32::UI::WindowsAndMessaging::SW_SHOW; + +const HOST_BINARY: &str = "DevolutionsHost.exe"; + +#[derive(Debug, Clone, Copy)] +pub enum SessionKind { + /// Console session. For example, when you connect to a user session on the local computer + /// by switching users on the computer. + Console, + /// Remote session. For example, when a user connects to a user session by using the Remote + /// Desktop Connection program from a remote computer. + Remote, +} + +struct GatewaySession { + session: Session, + kind: SessionKind, + is_host_ready: bool, +} + +// NOTE: `ceviche::controller::Session` do not implement `Debug` for session. +impl Debug for GatewaySession { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("GatewaySession") + .field("session", &self.session.id) + .field("kind", &self.kind) + .field("is_host_ready", &self.is_host_ready) + .finish() + } +} + +impl GatewaySession { + fn new(session: Session, kind: SessionKind) -> Self { + Self { + session, + kind, + is_host_ready: false, + } + } + + fn kind(&self) -> SessionKind { + self.kind + } + + fn os_session(&self) -> &Session { + &self.session + } + + fn is_host_ready(&self) -> bool { + self.is_host_ready + } + + fn set_host_ready(&mut self, is_ready: bool) { + self.is_host_ready = is_ready; + } +} + +#[derive(Default, Debug)] +struct SessionManagerCtx { + sessions: BTreeMap, +} + +impl SessionManagerCtx { + fn register_session(&mut self, session: &Session, kind: SessionKind) { + self.sessions + .insert(session.to_string(), GatewaySession::new(Session::new(session.id), kind)); + } + + fn unregister_session(&mut self, session: &Session) { + self.sessions.remove(&session.to_string()); + } + + fn get_session(&self, session: &Session) -> Option<&GatewaySession> { + self.sessions.get(&session.to_string()) + } + + fn get_session_mut(&mut self, session: &Session) -> Option<&mut GatewaySession> { + self.sessions.get_mut(&session.to_string()) + } +} + +pub struct SessionManager { + ctx: RwLock, + service_event_tx: mpsc::UnboundedSender, + service_event_rx: mpsc::UnboundedReceiver, +} + +impl Default for SessionManager { + fn default() -> Self { + let (service_event_tx, service_event_rx) = mpsc::unbounded_channel(); + + Self { + ctx: RwLock::new(SessionManagerCtx::default()), + service_event_tx, + service_event_rx, + } + } +} + +impl SessionManager { + pub(crate) fn service_event_tx(&self) -> mpsc::UnboundedSender { + self.service_event_tx.clone() + } +} + +#[async_trait] +impl Task for SessionManager { + type Output = anyhow::Result<()>; + + const NAME: &'static str = "SessionManager"; + + async fn run(self, mut shutdown_signal: ShutdownSignal) -> anyhow::Result<()> { + let Self { + mut service_event_rx, + ctx, + .. + } = self; + + info!("Starting session manager..."); + + loop { + tokio::select! { + event = service_event_rx.recv() => { + info!("Received service event"); + + let event = if let Some(event) = event { + event + } else { + error!("Service event channel closed"); + // Channel closed, all senders were dropped + break; + }; + + match event { + AgentServiceEvent::SessionConnect(id) => { + info!(%id, "Session connected"); + let mut ctx = ctx.write().await; + ctx.register_session(&id, SessionKind::Console); + // We only start the host process for remote sessions (initiated + // via RDP), as Host process with DVC handler is only needed for remote + // sessions. + } + AgentServiceEvent::SessionDisconnect(id) => { + info!(%id, "Session disconnected"); + try_terminate_host_process(&id); + ctx.write().await.unregister_session(&id); + } + AgentServiceEvent::SessionRemoteConnect(id) => { + info!(%id, "Remote session connected"); + // Terminate old host process if it is already running + try_terminate_host_process(&id); + + { + let mut ctx = ctx.write().await; + ctx.register_session(&id, SessionKind::Remote); + try_start_host_process(&mut ctx, &id)?; + } + } + AgentServiceEvent::SessionRemoteDisconnect(id) => { + info!(%id, "Remote session disconnected"); + // Terminate host process when remote session is disconnected + // (NOTE: depending on the system settings, session could + // still be running in the background after RDP disconnect) + try_terminate_host_process(&id); + ctx.write().await.unregister_session(&id); + } + AgentServiceEvent::SessionLogon(id) => { + info!(%id, "Session logged on"); + + // Terminate old host process if it is already running + try_terminate_host_process(&id); + + + // NOTE: In some cases, SessionRemoteConnect is fired before + // an actual user is logged in, therefore we need to try start the host + // app on logon, if not yet started + let mut ctx = ctx.write().await; + try_start_host_process(&mut ctx, &id)?; + } + AgentServiceEvent::SessionLogoff(id) => { + info!(%id, "Session logged off"); + ctx.write().await.get_session_mut(&id).map(|session| { + // When a user logs off, host process will be stopped by the system; + // Console sessions could be reused for different users, therefore + // we should not remove the session from the list, but mark it as + // not yet ready (host will be started as soon as new user logs in). + session.set_host_ready(false); + }); + } + _ => { + continue; + } + } + } + _ = shutdown_signal.wait() => { + info!("Shutting down session manager"); + break; + } + } + } + + Ok(()) + } +} + +/// Starts Devolutions Host process in the target session. +fn try_start_host_process(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { + match ctx.get_session_mut(&session) { + Some(gw_session) => { + if is_host_running_in_session(&session)? { + gw_session.set_host_ready(true); + return Ok(()); + } + + info!(%session, "Starting host process in session"); + + match start_host_process(&session) { + Ok(()) => { + info!(%session, "Host process started in session"); + gw_session.set_host_ready(true); + } + Err(err) => { + error!(%err, %session, "Failed to start host process for session"); + } + } + } + None => { + warn!(%session, "Session is not yet registered"); + } + }; + + Ok(()) +} + +/// Terminates Devolutions Host process in the target session. +fn try_terminate_host_process(session: &Session) { + match terminate_process_by_name_in_session(HOST_BINARY, session.id) { + Ok(false) => { + trace!(%session, "Host process is not running in the session"); + } + Ok(true) => { + info!(%session, "Host process terminated in session"); + } + Err(err) => { + error!(%err, %session, "Failed to terminate host process in session"); + } + } +} + +fn is_host_running_in_session(session: &Session) -> anyhow::Result { + let is_running = is_process_running_in_session(HOST_BINARY, session.id)?; + Ok(is_running) +} + +fn start_host_process(session: &Session) -> anyhow::Result<()> { + if !session_has_logged_in_user(session.id)? { + anyhow::bail!("Session {} does not have a logged in user", session); + } + + let host_app_path = host_app_path(); + let command_line = CommandLine::new(vec!["--session".to_owned(), session.to_string()]); + + info!("Starting `{host_app_path}` in session `{session}`"); + + let mut startup_info = StartupInfo::default(); + + // Run with GUI access + startup_info.show_window = SW_SHOW.0 as u16; + startup_info.flags = STARTF_USESHOWWINDOW; + startup_info.desktop = WideString::from("WinSta0\\Default"); + + let start_result = create_process_in_session( + session.id, + Some(host_app_path.as_std_path()), + Some(&command_line), + None, + None, + false, + CREATE_NEW_CONSOLE | NORMAL_PRIORITY_CLASS | CREATE_UNICODE_ENVIRONMENT, + None, + None, + &mut startup_info, + ); + + match start_result { + Ok(_) => { + info!("{HOST_BINARY} started in session {session}"); + Ok(()) + } + Err(err) => { + error!(%err, "Failed to start {HOST_BINARY} in session {session}"); + Err(err) + } + } +} + +fn host_app_path() -> Utf8PathBuf { + let mut current_dir = Utf8PathBuf::from_path_buf(std::env::current_exe().expect("BUG: can't get current exe path")) + .expect("BUG: OS should always return valid UTF-8 executable path"); + + current_dir.pop(); + current_dir.push(HOST_BINARY); + + current_dir +} diff --git a/devolutions-gateway/src/tls.rs b/devolutions-gateway/src/tls.rs index 6ab4ca913..b4bfc3bc0 100644 --- a/devolutions-gateway/src/tls.rs +++ b/devolutions-gateway/src/tls.rs @@ -276,8 +276,7 @@ pub mod sanity { mod danger { use tokio_rustls::rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; - use tokio_rustls::rustls::pki_types; - use tokio_rustls::rustls::{DigitallySignedStruct, Error, SignatureScheme}; + use tokio_rustls::rustls::{pki_types, DigitallySignedStruct, Error, SignatureScheme}; #[derive(Debug)] pub(super) struct NoCertificateVerification; diff --git a/devolutions-host/Cargo.toml b/devolutions-host/Cargo.toml new file mode 100644 index 000000000..3101d282b --- /dev/null +++ b/devolutions-host/Cargo.toml @@ -0,0 +1,38 @@ +[package] +name = "devolutions-host" +version = "2024.3.1" +edition = "2021" +license = "MIT/Apache-2.0" +authors = ["Devolutions Inc. "] +description = "Per-session host application for Devolutions Agent" +build = "build.rs" +publish = false + +[dependencies] +anyhow = "1.0" +camino = { version = "1.1", features = ["serde1"] } +cfg-if = "1" +ctrlc = "3.4" +devolutions-log = { path = "../crates/devolutions-log" } +parking_lot = "0.12" +serde = "1" +serde_json = "1" +tap = "1.0" +tracing = "0.1" + +[lints] +workspace = true + +[target.'cfg(windows)'.build-dependencies] +embed-resource = "2.4" + +[target.'cfg(windows)'.dependencies.windows] +version = "0.58" +features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_RemoteDesktop", + "Win32_System_IO", + "Win32_System_Threading", + "Win32_Security", +] \ No newline at end of file diff --git a/devolutions-host/build.rs b/devolutions-host/build.rs new file mode 100644 index 000000000..839115979 --- /dev/null +++ b/devolutions-host/build.rs @@ -0,0 +1,84 @@ +fn main() { + #[cfg(target_os = "windows")] + win::embed_version_rc(); +} + +#[cfg(target_os = "windows")] +mod win { + use std::{env, fs}; + + pub(super) fn embed_version_rc() { + let out_dir = env::var("OUT_DIR").unwrap(); + let version_rc_file = format!("{}/version.rc", out_dir); + let version_rc_data = generate_version_rc(); + fs::write(&version_rc_file, version_rc_data).unwrap(); + + embed_resource::compile(&version_rc_file, embed_resource::NONE); + } + + fn generate_version_rc() -> String { + let output_name = "DevolutionsHost"; + let filename = format!("{}.exe", output_name); + let company_name = "Devolutions Inc."; + let legal_copyright = format!("Copyright 2020-2024 {}", company_name); + + let version_number = env::var("CARGO_PKG_VERSION").unwrap() + ".0"; + let version_commas = version_number.replace('.', ","); + let file_description = output_name; + let file_version = version_number.clone(); + let internal_name = filename.clone(); + let original_filename = filename; + let product_name = output_name; + let product_version = version_number; + let vs_file_version = version_commas.clone(); + let vs_product_version = version_commas; + + let version_rc = format!( + r#"#include +VS_VERSION_INFO VERSIONINFO + FILEVERSION {vs_file_version} + PRODUCTVERSION {vs_product_version} + FILEFLAGSMASK 0x3fL +#ifdef _DEBUG + FILEFLAGS 0x1L +#else + FILEFLAGS 0x0L +#endif + FILEOS 0x40004L + FILETYPE 0x1L + FILESUBTYPE 0x0L +BEGIN + BLOCK "StringFileInfo" + BEGIN + BLOCK "040904b0" + BEGIN + VALUE "CompanyName", "{company_name}" + VALUE "FileDescription", "{file_description}" + VALUE "FileVersion", "{file_version}" + VALUE "InternalName", "{internal_name}" + VALUE "LegalCopyright", "{legal_copyright}" + VALUE "OriginalFilename", "{original_filename}" + VALUE "ProductName", "{product_name}" + VALUE "ProductVersion", "{product_version}" + END + END + BLOCK "VarFileInfo" + BEGIN + VALUE "Translation", 0x409, 1200 + END +END"#, + vs_file_version = vs_file_version, + vs_product_version = vs_product_version, + company_name = company_name, + file_description = file_description, + file_version = file_version, + internal_name = internal_name, + legal_copyright = legal_copyright, + original_filename = original_filename, + product_name = product_name, + product_version = product_version + ); + + version_rc + } +} diff --git a/devolutions-host/src/config.rs b/devolutions-host/src/config.rs new file mode 100644 index 000000000..f4dc37234 --- /dev/null +++ b/devolutions-host/src/config.rs @@ -0,0 +1,260 @@ +use anyhow::Context; +use camino::{Utf8Path, Utf8PathBuf}; +use cfg_if::cfg_if; +use serde::{Deserialize, Serialize}; +use tap::prelude::*; + +use std::fs::File; +use std::io::BufReader; +use std::sync::Arc; + +cfg_if! { + if #[cfg(target_os = "windows")] { + const COMPANY_DIR: &str = "Devolutions"; + const PROGRAM_DIR: &str = "Host"; + const APPLICATION_DIR: &str = "Devolutions\\Host"; + } else if #[cfg(target_os = "macos")] { + const COMPANY_DIR: &str = "Devolutions"; + const PROGRAM_DIR: &str = "Host"; + const APPLICATION_DIR: &str = "Devolutions Host"; + } else { + const COMPANY_DIR: &str = "devolutions"; + const PROGRAM_DIR: &str = "Host"; + const APPLICATION_DIR: &str = "devolutions-host"; + } +} + +#[derive(Debug, Clone)] +pub struct Conf { + pub log_file: Utf8PathBuf, + pub verbosity_profile: dto::VerbosityProfile, + pub debug: dto::DebugConf, +} + +impl Conf { + pub fn from_conf_file(conf_file: &dto::ConfFile) -> anyhow::Result { + let data_dir = get_data_dir(); + + let log_file = conf_file + .log_file + .clone() + .unwrap_or_else(|| Utf8PathBuf::from("host")) + .pipe_ref(|path| normalize_data_path(path, &data_dir)); + + Ok(Conf { + log_file, + verbosity_profile: conf_file.verbosity_profile.unwrap_or_default(), + debug: conf_file.debug.clone().unwrap_or_default(), + }) + } +} + +/// Configuration Handle, source of truth for current configuration state +#[derive(Clone)] +pub struct ConfHandle { + inner: Arc, +} + +struct ConfHandleInner { + conf: parking_lot::RwLock>, + conf_file: parking_lot::RwLock>, +} + +impl ConfHandle { + /// Initializes configuration for this instance. + /// + /// It's best to call this only once to avoid inconsistencies. + pub fn init() -> anyhow::Result { + let conf_file = load_conf_file_or_generate_new()?; + let conf = Conf::from_conf_file(&conf_file).context("invalid configuration file")?; + + Ok(Self { + inner: Arc::new(ConfHandleInner { + conf: parking_lot::RwLock::new(Arc::new(conf)), + conf_file: parking_lot::RwLock::new(Arc::new(conf_file)), + }), + }) + } + + /// Returns current configuration state (do not hold it forever as it may become outdated) + pub fn get_conf(&self) -> Arc { + self.inner.conf.read().clone() + } + + /// Returns current configuration file state (do not hold it forever as it may become outdated) + pub fn get_conf_file(&self) -> Arc { + self.inner.conf_file.read().clone() + } +} + +fn save_config(conf: &dto::ConfFile) -> anyhow::Result<()> { + let conf_file_path = get_conf_file_path(); + let json = serde_json::to_string_pretty(conf).context("failed JSON serialization of configuration")?; + std::fs::write(&conf_file_path, json).with_context(|| format!("failed to write file at {conf_file_path}"))?; + Ok(()) +} + +fn get_conf_file_path() -> Utf8PathBuf { + get_data_dir().join("agent.json") +} + +fn normalize_data_path(path: &Utf8Path, data_dir: &Utf8Path) -> Utf8PathBuf { + if path.is_absolute() { + path.to_owned() + } else { + data_dir.join(path) + } +} + +fn load_conf_file(conf_path: &Utf8Path) -> anyhow::Result> { + match File::open(conf_path) { + Ok(file) => BufReader::new(file) + .pipe(serde_json::from_reader) + .map(Some) + .with_context(|| format!("invalid config file at {conf_path}")), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(anyhow::anyhow!(e).context(format!("couldn't open config file at {conf_path}"))), + } +} + +#[allow(clippy::print_stdout)] // Logger is likely not yet initialized at this point, so it’s fine to write to stdout. +pub(crate) fn load_conf_file_or_generate_new() -> anyhow::Result { + let conf_file_path = get_conf_file_path(); + + let conf_file = match load_conf_file(&conf_file_path).context("failed to load configuration")? { + Some(conf_file) => conf_file, + None => { + let defaults = dto::ConfFile::generate_new(); + println!("Write default configuration to disk…"); + save_config(&defaults).context("failed to save configuration")?; + defaults + } + }; + + Ok(conf_file) +} + +pub(crate) mod dto { + use super::*; + + /// Source of truth for Agent configuration + /// + /// This struct represents the JSON file used for configuration as close as possible + /// and is not trying to be too smart. + /// + /// Unstable options are subject to change + #[derive(PartialEq, Debug, Clone, Serialize, Deserialize)] + #[serde(rename_all = "PascalCase")] + pub struct ConfFile { + /// Verbosity profile + #[serde(skip_serializing_if = "Option::is_none")] + pub verbosity_profile: Option, + + /// (Unstable) Folder and prefix for log files + #[serde(skip_serializing_if = "Option::is_none")] + pub log_file: Option, + + /// (Unstable) Unsafe debug options for developers + #[serde(rename = "__debug__", skip_serializing_if = "Option::is_none")] + pub debug: Option, + + /// Other unofficial options. + /// This field is useful so that we can deserialize + /// and then losslessly serialize back all root keys of the config file. + #[serde(flatten)] + pub rest: serde_json::Map, + } + + impl ConfFile { + pub fn generate_new() -> Self { + Self { + verbosity_profile: None, + log_file: None, + debug: None, + rest: serde_json::Map::new(), + } + } + } + + /// Verbosity profile (pre-defined tracing directives) + #[derive(PartialEq, Eq, Debug, Clone, Copy, Serialize, Deserialize, Default)] + pub enum VerbosityProfile { + /// The default profile, mostly info records + #[default] + Default, + /// Recommended profile for developers + Debug, + /// Show all traces + All, + /// Only show warnings and errors + Quiet, + } + + impl VerbosityProfile { + pub fn to_log_filter(self) -> &'static str { + match self { + VerbosityProfile::Default => "info", + VerbosityProfile::Debug => "info,devolutions_agent=debug", + VerbosityProfile::All => "trace", + VerbosityProfile::Quiet => "warn", + } + } + } + + /// Unsafe debug options that should only ever be used at development stage + /// + /// These options might change or get removed without further notice. + /// + /// Note to developers: all options should be safe by default, never add an option + /// that needs to be overridden manually in order to be safe. + #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] + pub struct DebugConf { + /// Directives string in the same form as the RUST_LOG environment variable + #[serde(skip_serializing_if = "Option::is_none")] + pub log_directives: Option, + + /// Enable unstable features which may break at any point + #[serde(default)] + pub enable_unstable: bool, + } + + /// Manual Default trait implementation just to make sure default values are deliberates + #[allow(clippy::derivable_impls)] + impl Default for DebugConf { + fn default() -> Self { + Self { + log_directives: None, + enable_unstable: false, + } + } + } + + impl DebugConf { + pub fn is_default(&self) -> bool { + Self::default().eq(self) + } + } +} + +pub fn get_data_dir() -> Utf8PathBuf { + if let Ok(config_path_env) = std::env::var("DHOST_DATA_PATH") { + Utf8PathBuf::from(config_path_env) + } else { + let mut config_path = Utf8PathBuf::new(); + + if cfg!(target_os = "windows") { + let program_data_env = std::env::var("APPDATA").expect("APPDATA env variable should be set on Windows"); + config_path.push(program_data_env); + config_path.push(COMPANY_DIR); + config_path.push(PROGRAM_DIR); + } else if cfg!(target_os = "macos") { + config_path.push("/Library/Application Support"); + config_path.push(APPLICATION_DIR); + } else { + config_path.push("/etc"); + config_path.push(APPLICATION_DIR); + } + + config_path + } +} diff --git a/devolutions-host/src/dvc.rs b/devolutions-host/src/dvc.rs new file mode 100644 index 000000000..240e2c20b --- /dev/null +++ b/devolutions-host/src/dvc.rs @@ -0,0 +1,147 @@ +//! WIP: This file is copied from MSRDPEX project + +use crate::config::ConfHandle; +use windows::core::PCSTR; +use windows::Win32::Foundation::{ + DuplicateHandle, GetLastError, DUPLICATE_SAME_ACCESS, ERROR_IO_PENDING, HANDLE, WIN32_ERROR, +}; +use windows::Win32::Storage::FileSystem::{ReadFile, WriteFile}; +use windows::Win32::System::RemoteDesktop::{ + WTSFreeMemory, WTSVirtualChannelClose, WTSVirtualChannelOpenEx, WTSVirtualChannelQuery, WTSVirtualFileHandle, + CHANNEL_FLAG_LAST, CHANNEL_PDU_HEADER, WTS_CHANNEL_OPTION_DYNAMIC, WTS_CURRENT_SESSION, WTS_VIRTUAL_CLASS, +}; +use windows::Win32::System::Threading::{CreateEventW, GetCurrentProcess, WaitForSingleObject, INFINITE}; +use windows::Win32::System::IO::{GetOverlappedResult, OVERLAPPED}; + +const CHANNEL_PDU_LENGTH: usize = 1024; + +pub fn loop_dvc(config: ConfHandle) { + if !config.get_conf().debug.enable_unstable { + debug!("DVC loop is disabled"); + return; + } + + info!("Starting DVC loop"); + + let channel_name = "DvcSample"; + match open_virtual_channel(channel_name) { + Ok(h_file) => { + info!("Virtual channel opened"); + + if let Err(err) = handle_virtual_channel(h_file) { + error!(%err, "DVC handling falied"); + } + } + Err(err) => { + error!(%err, "Failed to open virtual channel"); + // NOTE: Not exiting the program here, as it is not the main functionality + } + } + + info!("DVC loop finished"); +} + +fn open_virtual_channel(channel_name: &str) -> windows::core::Result { + unsafe { + let channel_name_wide = PCSTR::from_raw(channel_name.as_ptr()); + let h_wts_handle = WTSVirtualChannelOpenEx(WTS_CURRENT_SESSION, channel_name_wide, WTS_CHANNEL_OPTION_DYNAMIC) + .map_err(|e| std::io::Error::from_raw_os_error(e.code().0))?; + + let mut vc_file_handle_ptr: *mut HANDLE = std::ptr::null_mut(); + let mut len: u32 = 0; + let wts_virtual_class: WTS_VIRTUAL_CLASS = WTSVirtualFileHandle; + WTSVirtualChannelQuery( + h_wts_handle, + wts_virtual_class, + &mut vc_file_handle_ptr as *mut _ as *mut _, + &mut len, + ) + .map_err(|e| std::io::Error::from_raw_os_error(e.code().0))?; + + let mut new_handle: HANDLE = HANDLE::default(); + let _duplicate_result = DuplicateHandle( + GetCurrentProcess(), + *vc_file_handle_ptr, + GetCurrentProcess(), + &mut new_handle, + 0, + false, + DUPLICATE_SAME_ACCESS, + ); + + WTSFreeMemory(vc_file_handle_ptr as *mut core::ffi::c_void); + let _ = WTSVirtualChannelClose(h_wts_handle); + + Ok(new_handle) + } +} + +fn write_virtual_channel_message(h_file: HANDLE, cb_size: u32, buffer: *const u8) -> windows::core::Result<()> { + unsafe { + let buffer_slice = std::slice::from_raw_parts(buffer, cb_size as usize); + let mut dw_written: u32 = 0; + WriteFile(h_file, Some(buffer_slice), Some(&mut dw_written), None) + } +} + +#[allow(clippy::cast_possible_wrap)] +#[allow(clippy::cast_ptr_alignment)] +fn handle_virtual_channel(h_file: HANDLE) -> windows::core::Result<()> { + unsafe { + let mut read_buffer = [0u8; CHANNEL_PDU_LENGTH]; + let mut overlapped = OVERLAPPED::default(); + let mut dw_read: u32 = 0; + + let cmd = "whoami\0"; + let cb_size = cmd.len() as u32; + write_virtual_channel_message(h_file, cb_size, cmd.as_ptr())?; + + let h_event = CreateEventW(None, false, false, None)?; + overlapped.hEvent = h_event; + + loop { + // Notice the wrapping of parameters in Some() + let result = ReadFile( + h_file, + Some(&mut read_buffer), + Some(&mut dw_read), + Some(&mut overlapped), + ); + + if let Err(e) = result { + if GetLastError() == WIN32_ERROR(ERROR_IO_PENDING.0) { + let _dw_status = WaitForSingleObject(h_event, INFINITE); + if !GetOverlappedResult(h_file, &mut overlapped, &mut dw_read, false).is_ok() { + return Err(windows::core::Error::from_win32()); + } + } else { + return Err(e); + } + } + + println!("read {} bytes", dw_read); + + let packet_size = dw_read as usize - std::mem::size_of::(); + let p_data = read_buffer + .as_ptr() + .offset(std::mem::size_of::() as isize) as *const u8; + + println!( + ">> {}", + std::str::from_utf8(std::slice::from_raw_parts(p_data, packet_size)).unwrap_or("Invalid UTF-8") + ); + + if dw_read == 0 + || ((*(p_data.offset(-(std::mem::size_of::() as isize)) + as *const CHANNEL_PDU_HEADER)) + .flags + & CHANNEL_FLAG_LAST) + != 0 + { + break; + } + } + + Ok(()) + } +} diff --git a/devolutions-host/src/lib.rs b/devolutions-host/src/lib.rs new file mode 100644 index 000000000..58c59021b --- /dev/null +++ b/devolutions-host/src/lib.rs @@ -0,0 +1,14 @@ +#[macro_use] +extern crate tracing; + +#[cfg(windows)] +mod dvc; + +mod config; +mod log; + +pub use config::{get_data_dir, ConfHandle}; +pub use log::init_log; + +#[cfg(windows)] +pub use dvc::loop_dvc; diff --git a/devolutions-host/src/log.rs b/devolutions-host/src/log.rs new file mode 100644 index 000000000..e66bc55a7 --- /dev/null +++ b/devolutions-host/src/log.rs @@ -0,0 +1,21 @@ +use crate::config::ConfHandle; +use devolutions_log::{LoggerGuard, StaticLogConfig}; + +pub(crate) struct HostLog; + +impl StaticLogConfig for HostLog { + const MAX_BYTES_PER_LOG_FILE: u64 = 3_000_000; // 3 MB; + const MAX_LOG_FILES: usize = 10; + const LOG_FILE_PREFIX: &'static str = "host"; +} + +pub fn init_log(config: ConfHandle) -> LoggerGuard { + let conf = config.get_conf(); + + devolutions_log::init::( + &conf.log_file, + conf.verbosity_profile.to_log_filter(), + conf.debug.log_directives.as_deref(), + ) + .expect("BUG: Failed to initialize log") +} diff --git a/devolutions-host/src/main.rs b/devolutions-host/src/main.rs new file mode 100644 index 000000000..b94f7a7a9 --- /dev/null +++ b/devolutions-host/src/main.rs @@ -0,0 +1,46 @@ +// Start the program without a console window. +// It has no effect on platforms other than Windows. +#![windows_subsystem = "windows"] + +#[macro_use] +extern crate tracing; + +use devolutions_host::{get_data_dir, init_log, ConfHandle}; + +#[cfg(windows)] +use devolutions_host::loop_dvc; + +use anyhow::Context; + +use std::sync::mpsc; + +fn main() -> anyhow::Result<()> { + // Ensure per-user data dir exists + + std::fs::create_dir_all(get_data_dir()).context("Failed to create data directory")?; + + let config = ConfHandle::init().context("Failed to initialize configuration")?; + + let _logger_guard = init_log(config.clone()); + + info!("Starting Devolutions Host"); + + // TMP: Copy-paste from MSRDPEX project for testing purposes + #[cfg(windows)] + loop_dvc(config); + + let (shutdown_tx, shutdown_rx) = mpsc::channel(); + + ctrlc::set_handler(move || { + info!("Ctrl-C received, exiting"); + shutdown_tx.send(()).expect("BUG: Failed to send shutdown signal"); + }) + .expect("BUG: Failed to set Ctrl-C handler"); + + info!("Waiting for shutdown signal"); + shutdown_rx.recv().expect("BUG: Shutdown signal was lost"); + + info!("Exiting Devolutions Host"); + + Ok(()) +} diff --git a/jetsocat/src/pipe.rs b/jetsocat/src/pipe.rs index 06a8ef948..e540b7cea 100644 --- a/jetsocat/src/pipe.rs +++ b/jetsocat/src/pipe.rs @@ -287,8 +287,7 @@ pub async fn open_pipe(mode: PipeMode, proxy_cfg: Option) -> Result #[instrument(skip_all)] pub async fn pipe(mut a: Pipe, mut b: Pipe) -> Result<()> { - use tokio::io::copy_bidirectional_with_sizes; - use tokio::io::AsyncWriteExt as _; + use tokio::io::{copy_bidirectional_with_sizes, AsyncWriteExt as _}; const BUF_SIZE: usize = 16 * 1024; diff --git a/jetsocat/src/utils.rs b/jetsocat/src/utils.rs index 546cb804c..e710b95bd 100644 --- a/jetsocat/src/utils.rs +++ b/jetsocat/src/utils.rs @@ -155,8 +155,7 @@ where + Send + 'static, { - use futures_util::SinkExt as _; - use futures_util::StreamExt as _; + use futures_util::{SinkExt as _, StreamExt as _}; let compat = stream .map(|item| { From fc5c39e9d7d0ff95c073723ebb638ac16ea4008d Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 27 Aug 2024 17:28:03 +0300 Subject: [PATCH 02/10] Updated tlk to build devolutions-host --- ci/tlk.ps1 | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/ci/tlk.ps1 b/ci/tlk.ps1 index 9f845ca64..61e11751c 100644 --- a/ci/tlk.ps1 +++ b/ci/tlk.ps1 @@ -122,7 +122,7 @@ function Get-DestinationSymbolFile { $DestinationSymbolsName = $(Split-Path $DestinationExecutable -LeafBase) + ".$($Target.SymbolsExtension)" $DestinationDirectory = Split-Path $DestinationExecutable -Parent - + Join-Path $DestinationDirectory $DestinationSymbolsName } @@ -345,6 +345,7 @@ class TlkRecipe if ($this.Target.IsWindows()) { $agentPackages += [TlkPackage]::new("devolutions-pedm-hook", "crates/devolutions-pedm-hook", $true) $agentPackages += [TlkPackage]::new("devolutions-pedm-shell-ext", "crates/devolutions-pedm-shell-ext", $true) + $agentPackages += [TlkPackage]::new("devolutions-host", "crates/devolutions-host", $false) } $agentPackages @@ -405,13 +406,13 @@ class TlkRecipe $CargoPackages = $this.CargoPackages() $CargoTarget = $this.Target.CargoTarget() $CargoProfile = $this.Target.CargoProfile - + $CargoOutputPath = "$($this.SourcePath)/target/${CargoTarget}/${CargoProfile}" foreach ($CargoPackage in $CargoPackages) { Push-Location Set-Location -Path $CargoPackage.Path - + $this.Cargo(@('build')) $SrcBinaryPath = "${CargoOutputPath}/$($CargoPackage.BinaryFileName($this.Target))" @@ -455,10 +456,10 @@ class TlkRecipe if (Test-Path Env:STRIP_EXECUTABLE) { $StripExecutable = $Env:STRIP_EXECUTABLE } - + & $StripExecutable $SrcBinaryPath | Out-Host } - + if ($DestinationExecutable) { Copy-Item -Path $SrcBinaryPath -Destination $DestinationExecutable } @@ -472,7 +473,7 @@ class TlkRecipe Push-Location Set-Location $CargoOutputPath - + $MakeAppxOutput = & 'MakeAppx.exe' 'pack' '/f' "${CargoPackagePath}/mapping.txt" '/p' "./DevolutionsPedmShellExt.msix" '/nv' '/o' if (!$?) { throw "MakeAppx package creation failed: ${MakeAppxOutput}" @@ -497,7 +498,7 @@ class TlkRecipe Copy-Item -Path $builtDesktopExe -Destination $Env:DAGENT_PEDM_DESKTOP_EXECUTABLE Copy-Item -Path $builtDesktopPdb -Destination $(Get-DestinationSymbolFile $Env:DAGENT_PEDM_DESKTOP_EXECUTABLE $this.Target) - + } } From 22f9a9feb8d91da764806d0f0b421c0518aebf2a Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 27 Aug 2024 17:29:43 +0300 Subject: [PATCH 03/10] Disable Devolutions host by default --- devolutions-agent/src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devolutions-agent/src/config.rs b/devolutions-agent/src/config.rs index 7dd740b9d..6695df93b 100644 --- a/devolutions-agent/src/config.rs +++ b/devolutions-agent/src/config.rs @@ -319,7 +319,7 @@ pub mod dto { remote_desktop: None, pedm: None, debug: None, - session_host: Some(SessionHostConf { enabled: true }), + session_host: Some(SessionHostConf { enabled: false }), rest: serde_json::Map::new(), } } From d0a6d57ebd0f2bda358d11e63e03be13e87bd3a7 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 27 Aug 2024 18:01:50 +0300 Subject: [PATCH 04/10] Fixed a bunch of lints --- crates/win-api-wrappers/src/process.rs | 30 ++++++++++--------- .../src/security/privilege.rs | 4 +-- devolutions-agent/build.rs | 7 +++-- devolutions-agent/src/config.rs | 1 + devolutions-agent/src/session_manager/mod.rs | 16 ++++++---- devolutions-host/build.rs | 7 +++-- devolutions-host/src/dvc.rs | 16 +++++++--- devolutions-host/src/main.rs | 2 +- 8 files changed, 50 insertions(+), 33 deletions(-) diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 10aca9cf1..0618647a9 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -572,7 +572,9 @@ impl ProcessEntry32Iterator { // SAFETY: It is safe to zero out the structure as it is a simple POD type. let mut process_entry: PROCESSENTRY32W = unsafe { mem::zeroed() }; - process_entry.dwSize = mem::size_of::().try_into().unwrap(); + process_entry.dwSize = mem::size_of::() + .try_into() + .expect("BUG: PROCESSENTRY32W size always fits in u32"); Ok(ProcessEntry32Iterator { snapshot_handle, @@ -625,7 +627,7 @@ impl ProcessEntry { .szExeFile .iter() .position(|&c| c == 0) - .ok_or(anyhow!("Executable name null terminator not found"))?; + .ok_or_else(|| anyhow!("Executable name null terminator not found"))?; let name = String::from_utf16(&self.0.szExeFile[..exe_name_length]) .map_err(|_| anyhow!("Invalid executable name UTF16 encoding"))?; @@ -650,18 +652,15 @@ impl ProcessEnvironment { impl Drop for ProcessEnvironment { fn drop(&mut self) { - match self { - ProcessEnvironment::OsDefined(block) => { - // SAFETY: `block` is checked to be valid before free. - unsafe { - if !block.is_null() { - if let Err(err) = DestroyEnvironmentBlock(*block) { - warn!(%err, "Failed to destroy environment block"); - } + if let ProcessEnvironment::OsDefined(block) = self { + // SAFETY: `block` is checked to be valid before free. + unsafe { + if !block.is_null() { + if let Err(err) = DestroyEnvironmentBlock(*block) { + warn!(%err, "Failed to destroy environment block"); } - }; - } - _ => {} + } + }; } } } @@ -689,10 +688,11 @@ pub fn create_process_as_user( let mut environment: *mut c_void = ptr::null_mut(); if let Some(token) = token { + // SAFETY: Allocated memory will be freed by `ProcessEnvironment` destructor. unsafe { CreateEnvironmentBlock(&mut environment, token.handle().raw(), false) }?; } - ProcessEnvironment::OsDefined(environment as *const _) + ProcessEnvironment::OsDefined(environment.cast_const()) }; let mut command_line = command_line @@ -743,6 +743,7 @@ pub fn create_process_as_user( /// Starts new process in the specified session. Note that this function requires the current /// process to have `SYSTEM`-level permissions. Use with caution. +#[allow(clippy::too_many_arguments)] pub fn create_process_in_session( session_id: u32, application_name: Option<&Path>, @@ -841,6 +842,7 @@ fn terminate_process_by_name_impl(process_name: &str, session_id: Option) - match process { Ok(process) => { + // SAFETY: `OpenProcess` ensures that the handle is valid. unsafe { if let Err(err) = TerminateProcess(process, 1) { warn!(process_name, session_id, %err, "TerminateProcess failed"); diff --git a/crates/win-api-wrappers/src/security/privilege.rs b/crates/win-api-wrappers/src/security/privilege.rs index e70ba6ef3..7cf4efe2f 100644 --- a/crates/win-api-wrappers/src/security/privilege.rs +++ b/crates/win-api-wrappers/src/security/privilege.rs @@ -146,11 +146,11 @@ impl<'a> ScopedPrivileges<'a> { } pub fn token(&self) -> &Token { - &self.token + self.token } pub fn token_mut(&mut self) -> &mut Token { - &mut self.token + self.token } } diff --git a/devolutions-agent/build.rs b/devolutions-agent/build.rs index 8c8bcedf8..a98c06e29 100644 --- a/devolutions-agent/build.rs +++ b/devolutions-agent/build.rs @@ -8,10 +8,10 @@ mod win { use std::{env, fs}; pub(super) fn embed_version_rc() { - let out_dir = env::var("OUT_DIR").unwrap(); + let out_dir = env::var("OUT_DIR").expect("BUG: failed to get OUT_DIR"); let version_rc_file = format!("{}/version.rc", out_dir); let version_rc_data = generate_version_rc(); - fs::write(&version_rc_file, version_rc_data).unwrap(); + fs::write(&version_rc_file, version_rc_data).expect("BUG: failed to write version.rc"); embed_resource::compile(&version_rc_file, embed_resource::NONE); embed_resource::compile("resources.rc", embed_resource::NONE); @@ -23,7 +23,8 @@ mod win { let company_name = "Devolutions Inc."; let legal_copyright = format!("Copyright 2020-2024 {}", company_name); - let version_number = env::var("CARGO_PKG_VERSION").unwrap() + ".0"; + let mut version_number = env::var("CARGO_PKG_VERSION").expect("BUG: failed to get CARGO_PKG_VERSION"); + version_number.push_str(".0"); let version_commas = version_number.replace('.', ","); let file_description = output_name; let file_version = version_number.clone(); diff --git a/devolutions-agent/src/config.rs b/devolutions-agent/src/config.rs index 6695df93b..0385bd78c 100644 --- a/devolutions-agent/src/config.rs +++ b/devolutions-agent/src/config.rs @@ -262,6 +262,7 @@ pub mod dto { pub enabled: bool, } + #[allow(clippy::derivable_impls)] // Just to be explicit about the default values of the config. impl Default for SessionHostConf { fn default() -> Self { Self { enabled: false } diff --git a/devolutions-agent/src/session_manager/mod.rs b/devolutions-agent/src/session_manager/mod.rs index 88d82577f..d5e0a816b 100644 --- a/devolutions-agent/src/session_manager/mod.rs +++ b/devolutions-agent/src/session_manager/mod.rs @@ -199,13 +199,13 @@ impl Task for SessionManager { } AgentServiceEvent::SessionLogoff(id) => { info!(%id, "Session logged off"); - ctx.write().await.get_session_mut(&id).map(|session| { + if let Some(mut session) = ctx.write().await.get_session_mut(&id) { // When a user logs off, host process will be stopped by the system; // Console sessions could be reused for different users, therefore // we should not remove the session from the list, but mark it as // not yet ready (host will be started as soon as new user logs in). session.set_host_ready(false); - }); + } } _ => { continue; @@ -225,16 +225,16 @@ impl Task for SessionManager { /// Starts Devolutions Host process in the target session. fn try_start_host_process(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { - match ctx.get_session_mut(&session) { + match ctx.get_session_mut(session) { Some(gw_session) => { - if is_host_running_in_session(&session)? { + if is_host_running_in_session(session)? { gw_session.set_host_ready(true); return Ok(()); } info!(%session, "Starting host process in session"); - match start_host_process(&session) { + match start_host_process(session) { Ok(()) => { info!(%session, "Host process started in session"); gw_session.set_host_ready(true); @@ -285,7 +285,11 @@ fn start_host_process(session: &Session) -> anyhow::Result<()> { let mut startup_info = StartupInfo::default(); // Run with GUI access - startup_info.show_window = SW_SHOW.0 as u16; + // NOTE: silent clippy warning, just to be more explicit about `show_window` value + #[allow(clippy::field_reassign_with_default)] + { + startup_info.show_window = u16::try_from(SW_SHOW.0).expect("BUG: SW_SHOW always fit u16"); + } startup_info.flags = STARTF_USESHOWWINDOW; startup_info.desktop = WideString::from("WinSta0\\Default"); diff --git a/devolutions-host/build.rs b/devolutions-host/build.rs index 839115979..0b96bd32e 100644 --- a/devolutions-host/build.rs +++ b/devolutions-host/build.rs @@ -8,10 +8,10 @@ mod win { use std::{env, fs}; pub(super) fn embed_version_rc() { - let out_dir = env::var("OUT_DIR").unwrap(); + let out_dir = env::var("OUT_DIR").expect("BUG: failed to get OUT_DIR"); let version_rc_file = format!("{}/version.rc", out_dir); let version_rc_data = generate_version_rc(); - fs::write(&version_rc_file, version_rc_data).unwrap(); + fs::write(&version_rc_file, version_rc_data).expect("BUG: failed to write version.rc"); embed_resource::compile(&version_rc_file, embed_resource::NONE); } @@ -22,7 +22,8 @@ mod win { let company_name = "Devolutions Inc."; let legal_copyright = format!("Copyright 2020-2024 {}", company_name); - let version_number = env::var("CARGO_PKG_VERSION").unwrap() + ".0"; + let mut version_number = env::var("CARGO_PKG_VERSION").expect("BUG: failed to get CARGO_PKG_VERSION"); + version_number.push_str(".0"); let version_commas = version_number.replace('.', ","); let file_description = output_name; let file_version = version_number.clone(); diff --git a/devolutions-host/src/dvc.rs b/devolutions-host/src/dvc.rs index 240e2c20b..695a70823 100644 --- a/devolutions-host/src/dvc.rs +++ b/devolutions-host/src/dvc.rs @@ -41,6 +41,8 @@ pub fn loop_dvc(config: ConfHandle) { info!("DVC loop finished"); } +#[allow(clippy::multiple_unsafe_ops_per_block)] +#[allow(clippy::undocumented_unsafe_blocks)] fn open_virtual_channel(channel_name: &str) -> windows::core::Result { unsafe { let channel_name_wide = PCSTR::from_raw(channel_name.as_ptr()); @@ -76,6 +78,8 @@ fn open_virtual_channel(channel_name: &str) -> windows::core::Result { } } +#[allow(clippy::multiple_unsafe_ops_per_block)] +#[allow(clippy::undocumented_unsafe_blocks)] fn write_virtual_channel_message(h_file: HANDLE, cb_size: u32, buffer: *const u8) -> windows::core::Result<()> { unsafe { let buffer_slice = std::slice::from_raw_parts(buffer, cb_size as usize); @@ -86,6 +90,10 @@ fn write_virtual_channel_message(h_file: HANDLE, cb_size: u32, buffer: *const u8 #[allow(clippy::cast_possible_wrap)] #[allow(clippy::cast_ptr_alignment)] +#[allow(clippy::ptr_offset_with_cast)] +#[allow(clippy::cast_possible_truncation)] +#[allow(clippy::multiple_unsafe_ops_per_block)] +#[allow(clippy::undocumented_unsafe_blocks)] fn handle_virtual_channel(h_file: HANDLE) -> windows::core::Result<()> { unsafe { let mut read_buffer = [0u8; CHANNEL_PDU_LENGTH]; @@ -111,7 +119,7 @@ fn handle_virtual_channel(h_file: HANDLE) -> windows::core::Result<()> { if let Err(e) = result { if GetLastError() == WIN32_ERROR(ERROR_IO_PENDING.0) { let _dw_status = WaitForSingleObject(h_event, INFINITE); - if !GetOverlappedResult(h_file, &mut overlapped, &mut dw_read, false).is_ok() { + if GetOverlappedResult(h_file, &overlapped, &mut dw_read, false).is_err() { return Err(windows::core::Error::from_win32()); } } else { @@ -119,14 +127,14 @@ fn handle_virtual_channel(h_file: HANDLE) -> windows::core::Result<()> { } } - println!("read {} bytes", dw_read); + info!("read {} bytes", dw_read); let packet_size = dw_read as usize - std::mem::size_of::(); let p_data = read_buffer .as_ptr() - .offset(std::mem::size_of::() as isize) as *const u8; + .offset(std::mem::size_of::() as isize); - println!( + info!( ">> {}", std::str::from_utf8(std::slice::from_raw_parts(p_data, packet_size)).unwrap_or("Invalid UTF-8") ); diff --git a/devolutions-host/src/main.rs b/devolutions-host/src/main.rs index b94f7a7a9..648394b3e 100644 --- a/devolutions-host/src/main.rs +++ b/devolutions-host/src/main.rs @@ -1,6 +1,6 @@ // Start the program without a console window. // It has no effect on platforms other than Windows. -#![windows_subsystem = "windows"] +#![cfg_attr(windows, windows_subsystem = "windows")] #[macro_use] extern crate tracing; From e53a0d34fce50dc78c68e98d8fb0fcee9a8ffedf Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Mon, 2 Sep 2024 10:01:55 +0300 Subject: [PATCH 05/10] refactoring: fixed minor comments --- Cargo.lock | 2 +- Cargo.toml | 5 +- ci/tlk.ps1 | 2 +- crates/win-api-wrappers/src/handle.rs | 3 +- crates/win-api-wrappers/src/process.rs | 72 +++++++++++++------ .../src/security/privilege.rs | 4 +- crates/win-api-wrappers/src/session.rs | 6 +- crates/win-api-wrappers/src/token.rs | 14 ++-- crates/win-api-wrappers/src/utils.rs | 19 ----- devolutions-agent/src/main.rs | 7 +- devolutions-agent/src/service.rs | 6 +- devolutions-agent/src/session_manager/mod.rs | 59 +++++++-------- devolutions-agent/src/updater/io.rs | 4 +- devolutions-agent/src/updater/mod.rs | 21 +++--- devolutions-agent/src/updater/package.rs | 4 +- devolutions-host/Cargo.toml | 2 +- devolutions-host/src/dvc.rs | 8 +-- devolutions-host/src/main.rs | 4 +- tools/bump-version.ps1 | 2 +- 19 files changed, 132 insertions(+), 112 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ee791a57..f050ec12a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1120,7 +1120,7 @@ dependencies = [ [[package]] name = "devolutions-host" -version = "2024.3.1" +version = "2024.3.2" dependencies = [ "anyhow", "camino", diff --git a/Cargo.toml b/Cargo.toml index 91f37a355..db39caac2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,14 +3,15 @@ resolver = "2" members = [ "crates/*", "devolutions-agent", - "devolutions-gateway", "devolutions-host", + "devolutions-gateway", + "devolutions-host", "jetsocat", "tools/generate-openapi", ] default-members = [ "devolutions-agent", "devolutions-gateway", - "devolutions-agent", + "devolutions-host", "jetsocat", ] diff --git a/ci/tlk.ps1 b/ci/tlk.ps1 index 61e11751c..0880476cd 100644 --- a/ci/tlk.ps1 +++ b/ci/tlk.ps1 @@ -345,7 +345,7 @@ class TlkRecipe if ($this.Target.IsWindows()) { $agentPackages += [TlkPackage]::new("devolutions-pedm-hook", "crates/devolutions-pedm-hook", $true) $agentPackages += [TlkPackage]::new("devolutions-pedm-shell-ext", "crates/devolutions-pedm-shell-ext", $true) - $agentPackages += [TlkPackage]::new("devolutions-host", "crates/devolutions-host", $false) + $agentPackages += [TlkPackage]::new("devolutions-host", "devolutions-host", $false) } $agentPackages diff --git a/crates/win-api-wrappers/src/handle.rs b/crates/win-api-wrappers/src/handle.rs index 4f8a8f05c..1843cf0fb 100644 --- a/crates/win-api-wrappers/src/handle.rs +++ b/crates/win-api-wrappers/src/handle.rs @@ -1,8 +1,7 @@ use std::fmt::Debug; use std::os::windows::io::{AsRawHandle, BorrowedHandle, IntoRawHandle, OwnedHandle}; -use windows::Win32::Foundation::E_HANDLE; -use windows::Win32::Foundation::{CloseHandle, DuplicateHandle, DUPLICATE_SAME_ACCESS, HANDLE}; +use windows::Win32::Foundation::{CloseHandle, DuplicateHandle, DUPLICATE_SAME_ACCESS, E_HANDLE, HANDLE}; use windows::Win32::System::Threading::GetCurrentProcess; // TODO: Use/implement AsHandle and AsRawHandle as appropriate diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 0618647a9..709a8a47b 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -6,18 +6,19 @@ use std::os::windows::ffi::OsStringExt; use std::path::{Path, PathBuf}; use std::{ptr, slice}; -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Context, Result}; use crate::handle::{Handle, HandleWrapper}; use crate::security::acl::{RawSecurityAttributes, SecurityAttributes}; use crate::thread::Thread; use crate::token::Token; use crate::undoc::{NtQueryInformationProcess, ProcessBasicInformation, RTL_USER_PROCESS_PARAMETERS}; -use crate::utils::{serialize_environment, Allocation, AnsiString, CommandLine, WideString}; +use crate::utils::{Allocation, AnsiString, CommandLine, WideString}; use crate::Error; use windows::core::PCWSTR; use windows::Win32::Foundation::{ - FreeLibrary, ERROR_INCORRECT_SIZE, HANDLE, HMODULE, MAX_PATH, WAIT_EVENT, WAIT_FAILED, + FreeLibrary, ERROR_INCORRECT_SIZE, ERROR_NO_MORE_FILES, E_INVALIDARG, HANDLE, HMODULE, MAX_PATH, WAIT_EVENT, + WAIT_FAILED, }; use windows::Win32::Security::{ SE_ASSIGNPRIMARYTOKEN_NAME, SE_INCREASE_QUOTA_NAME, SE_TCB_NAME, TOKEN_ACCESS_MASK, TOKEN_ADJUST_PRIVILEGES, @@ -566,9 +567,14 @@ pub struct ProcessEntry32Iterator { impl ProcessEntry32Iterator { pub fn new() -> Result { - // SAFETY: `CreateToolhelp32Snapshot` call is always safe and `Owned` wrapper ensures - // that the handle is properly closed when the iterator is dropped. - let snapshot_handle = unsafe { Handle::new(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0)?, true) }; + // SAFETY: `CreateToolhelp32Snapshot` call is always safe to call and returns a + // valid handle on success. + let raw_handle = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }?; + + // SAFETY: `Handle::new` is safe to call because the `raw_handle` is valid. + let snapshot_handle = unsafe { + Handle::new_owned(raw_handle).expect("BUG: handle should be valid after CreateToolhelp32Snapshot call") + }; // SAFETY: It is safe to zero out the structure as it is a simple POD type. let mut process_entry: PROCESSENTRY32W = unsafe { mem::zeroed() }; @@ -590,7 +596,7 @@ impl Iterator for ProcessEntry32Iterator { fn next(&mut self) -> Option { let result = if self.first { // SAFETY: `windows` library ensures that `snapshot` handle is correct on creation, - // therefore it is safe to call Process32First + // therefore it is safe to call Process32First. unsafe { Process32FirstW(self.snapshot_handle.raw(), &mut self.process_entry as *mut _) } } else { // SAFETY: `Process32Next` is safe to call because the `snapshot_handle` is valid while it @@ -599,9 +605,9 @@ impl Iterator for ProcessEntry32Iterator { }; match result { - Err(err) if err.code() == ERROR_NO_MORE_FILES.to_hresult() => None, - Err(err) => { - error!(%err, "Failed to iterate over processes"); + Err(error) if error.code() == ERROR_NO_MORE_FILES.to_hresult() => None, + Err(error) => { + error!(%error, "Failed to iterate over processes"); None } Ok(()) => { @@ -627,10 +633,10 @@ impl ProcessEntry { .szExeFile .iter() .position(|&c| c == 0) - .ok_or_else(|| anyhow!("Executable name null terminator not found"))?; + .context("executable name null terminator not found")?; let name = String::from_utf16(&self.0.szExeFile[..exe_name_length]) - .map_err(|_| anyhow!("Invalid executable name UTF16 encoding"))?; + .context("invalid executable name UTF16 encoding")?; Ok(name) } @@ -653,11 +659,12 @@ impl ProcessEnvironment { impl Drop for ProcessEnvironment { fn drop(&mut self) { if let ProcessEnvironment::OsDefined(block) = self { - // SAFETY: `block` is checked to be valid before free. + // SAFETY: `ProcessEnvironment` is a private enum, and we ensured that `block` will only + // ever hold pointers returned by `CreateEnvironmentBlock` in the current module. unsafe { if !block.is_null() { - if let Err(err) = DestroyEnvironmentBlock(*block) { - warn!(%err, "Failed to destroy environment block"); + if let Err(error) = DestroyEnvironmentBlock(*block) { + warn!(%error, "Failed to destroy environment block"); } } }; @@ -688,7 +695,13 @@ pub fn create_process_as_user( let mut environment: *mut c_void = ptr::null_mut(); if let Some(token) = token { - // SAFETY: Allocated memory will be freed by `ProcessEnvironment` destructor. + // SAFETY: As per `CreateEnvironmentBlock` documentation: We must specify + // `CREATE_UNICODE_ENVIRONMENT` and call `DestroyEnvironmentBlock` after + // `CreateProcessAsUser` call. + // - `CREATE_UNICODE_ENVIRONMENT` is always set unconditionaly. + // - `DestroyEnvironmentBlock` is called in the `ProcessEnvironment` destructor. + // + // Therefore, all preconditions are met to safely call `CreateEnvironmentBlock`. unsafe { CreateEnvironmentBlock(&mut environment, token.handle().raw(), false) }?; } @@ -741,6 +754,25 @@ pub fn create_process_as_user( }) } +fn serialize_environment(environment: &HashMap) -> Result> { + let mut serialized = Vec::new(); + + for (k, v) in environment.iter() { + if k.contains('=') { + bail!(Error::from_hresult(E_INVALIDARG)); + } + + serialized.extend(k.encode_utf16()); + serialized.extend("=".encode_utf16()); + serialized.extend(v.encode_utf16()); + serialized.push(0); + } + + serialized.push(0); + + Ok(serialized) +} + /// Starts new process in the specified session. Note that this function requires the current /// process to have `SYSTEM`-level permissions. Use with caution. #[allow(clippy::too_many_arguments)] @@ -844,16 +876,16 @@ fn terminate_process_by_name_impl(process_name: &str, session_id: Option) - Ok(process) => { // SAFETY: `OpenProcess` ensures that the handle is valid. unsafe { - if let Err(err) = TerminateProcess(process, 1) { - warn!(process_name, session_id, %err, "TerminateProcess failed"); + if let Err(error) = TerminateProcess(process, 1) { + warn!(process_name, session_id, %error, "TerminateProcess failed"); return Ok(false); } } return Ok(true); } - Err(err) => { - warn!(process_name, session_id, %err, "OpenProcess failed"); + Err(error) => { + warn!(process_name, session_id, %error, "OpenProcess failed"); continue; } } diff --git a/crates/win-api-wrappers/src/security/privilege.rs b/crates/win-api-wrappers/src/security/privilege.rs index 7cf4efe2f..a914fe0d8 100644 --- a/crates/win-api-wrappers/src/security/privilege.rs +++ b/crates/win-api-wrappers/src/security/privilege.rs @@ -156,11 +156,11 @@ impl<'a> ScopedPrivileges<'a> { impl Drop for ScopedPrivileges<'_> { fn drop(&mut self) { - if let Err(err) = self + if let Err(error) = self .token .adjust_privileges(&TokenPrivilegesAdjustment::Disable(self.token_privileges.clone())) { - error!(%err, "Failed to disable ScopedPrivileges({})", self.description); + error!(%error, "Failed to disable ScopedPrivileges({})", self.description); } } } diff --git a/crates/win-api-wrappers/src/session.rs b/crates/win-api-wrappers/src/session.rs index 31a96e4cb..d1024141f 100644 --- a/crates/win-api-wrappers/src/session.rs +++ b/crates/win-api-wrappers/src/session.rs @@ -1,8 +1,7 @@ -use super::handle::Handle; use super::process::Process; use super::security::privilege::ScopedPrivileges; use anyhow::Result; -use windows::Win32::Foundation::{ERROR_NO_TOKEN, HANDLE}; +use windows::Win32::Foundation::{CloseHandle, ERROR_NO_TOKEN, HANDLE}; use windows::Win32::Security::{SE_TCB_NAME, TOKEN_ADJUST_PRIVILEGES, TOKEN_QUERY}; use windows::Win32::System::RemoteDesktop::WTSQueryUserToken; @@ -20,7 +19,8 @@ pub fn session_has_logged_in_user(session_id: u32) -> Result { Err(err) => Err(err.into()), Ok(()) => { // Close handle immediately. - let _handle: Handle = Handle::new(handle, true); + // SAFETY: `CloseHandle` is safe to call with a valid handle. + unsafe { CloseHandle(handle).expect("BUG: WTSQueryUserToken should return a valid handle") }; Ok(true) } } diff --git a/crates/win-api-wrappers/src/token.rs b/crates/win-api-wrappers/src/token.rs index ecc44c512..fe5576516 100644 --- a/crates/win-api-wrappers/src/token.rs +++ b/crates/win-api-wrappers/src/token.rs @@ -24,7 +24,7 @@ use crate::undoc::{ TOKEN_SECURITY_ATTRIBUTE_TYPE_OCTET_STRING, TOKEN_SECURITY_ATTRIBUTE_TYPE_STRING, TOKEN_SECURITY_ATTRIBUTE_TYPE_UINT64, TOKEN_SECURITY_ATTRIBUTE_V1, TOKEN_SECURITY_ATTRIBUTE_V1_VALUE, }; -use crate::utils::WideString; +use crate::utils::{size_of_u32, WideString}; use crate::{create_impersonation_context, Error}; use windows::Win32::Foundation::{ ERROR_ALREADY_EXISTS, ERROR_INVALID_SECURITY_DESCR, ERROR_INVALID_VARIANT, ERROR_NO_TOKEN, ERROR_SUCCESS, HANDLE, @@ -42,10 +42,8 @@ use windows::Win32::Security::{ TOKEN_INFORMATION_CLASS, TOKEN_MANDATORY_POLICY, TOKEN_MANDATORY_POLICY_ID, TOKEN_OWNER, TOKEN_PRIMARY_GROUP, TOKEN_PRIVILEGES, TOKEN_PRIVILEGES_ATTRIBUTES, TOKEN_QUERY, TOKEN_SOURCE, TOKEN_STATISTICS, TOKEN_TYPE, TOKEN_USER, }; -use windows::Win32::System::SystemServices::SE_GROUP_LOGON_ID; - -use super::utils::size_of_u32; use windows::Win32::System::RemoteDesktop::WTSQueryUserToken; +use windows::Win32::System::SystemServices::SE_GROUP_LOGON_ID; #[derive(Debug)] pub struct Token { @@ -68,11 +66,15 @@ impl Token { pub fn for_session(session_id: u32) -> Result { let mut user_token = HANDLE::default(); - // SAFETY: query user token is always safe if dst pointer is valid. + // SAFETY: Query user token is always safe if dst pointer is valid. unsafe { WTSQueryUserToken(session_id, &mut user_token as *mut _)? }; Ok(Self { - handle: user_token.into(), + // SAFETY: As per `WTSQueryUserToken` documentation, returned token should be closed + // by caller. + handle: unsafe { + Handle::new_owned(user_token).expect("BUG: WTSQueryUserToken should return a valid handle") + }, }) } diff --git a/crates/win-api-wrappers/src/utils.rs b/crates/win-api-wrappers/src/utils.rs index b991dffbd..ea50b7498 100644 --- a/crates/win-api-wrappers/src/utils.rs +++ b/crates/win-api-wrappers/src/utils.rs @@ -343,25 +343,6 @@ pub unsafe fn set_memory_protection( Ok(old_prot) } -pub fn serialize_environment(environment: &HashMap) -> Result> { - let mut serialized = Vec::new(); - - for (k, v) in environment.iter() { - if k.contains('=') { - bail!(Error::from_hresult(E_INVALIDARG)); - } - - serialized.extend(k.encode_utf16()); - serialized.extend("=".encode_utf16()); - serialized.extend(v.encode_utf16()); - serialized.push(0); - } - - serialized.push(0); - - Ok(serialized) -} - pub fn environment_block(token: Option<&Token>, inherit: bool) -> Result> { let mut blocks = Vec::new(); diff --git a/devolutions-agent/src/main.rs b/devolutions-agent/src/main.rs index 1622ac50d..dc6303d5e 100644 --- a/devolutions-agent/src/main.rs +++ b/devolutions-agent/src/main.rs @@ -60,6 +60,7 @@ fn agent_service_main( match control_code { AgentServiceEvent::Stop => { + service.stop(); break; } AgentServiceEvent::SessionConnect(_) @@ -69,10 +70,10 @@ fn agent_service_main( | AgentServiceEvent::SessionLogon(_) | AgentServiceEvent::SessionLogoff(_) => { if let Some(tx) = service_event_tx.as_mut() { - match tx.send(control_code) { + match tx.blocking_send(control_code) { Ok(()) => {} - Err(err) => { - error!(%err, "Failed to send event to session manager"); + Err(error) => { + error!(%error, "Failed to send event to session manager"); service_event_tx = None; } } diff --git a/devolutions-agent/src/service.rs b/devolutions-agent/src/service.rs index a68f24f11..309bbc9e3 100644 --- a/devolutions-agent/src/service.rs +++ b/devolutions-agent/src/service.rs @@ -24,7 +24,7 @@ struct TasksCtx { /// Spawned service tasks tasks: Tasks, /// Sender side of the service event channel (Used for session manager module) - service_event_tx: Option>, + service_event_tx: Option>, } #[allow(clippy::large_enum_variant)] // `Running` variant is bigger than `Stopped` but we don't care @@ -40,7 +40,7 @@ pub struct AgentService { conf_handle: ConfHandle, state: AgentState, _logger_guard: LoggerGuard, - service_event_tx: Option>, + service_event_tx: Option>, } impl AgentService { @@ -165,7 +165,7 @@ impl AgentService { } } - pub fn service_event_tx(&self) -> Option> { + pub fn service_event_tx(&self) -> Option> { self.service_event_tx.clone() } } diff --git a/devolutions-agent/src/session_manager/mod.rs b/devolutions-agent/src/session_manager/mod.rs index d5e0a816b..b308c59af 100644 --- a/devolutions-agent/src/session_manager/mod.rs +++ b/devolutions-agent/src/session_manager/mod.rs @@ -23,7 +23,7 @@ use windows::Win32::UI::WindowsAndMessaging::SW_SHOW; const HOST_BINARY: &str = "DevolutionsHost.exe"; #[derive(Debug, Clone, Copy)] -pub enum SessionKind { +pub(crate) enum SessionKind { /// Console session. For example, when you connect to a user session on the local computer /// by switching users on the computer. Console, @@ -58,14 +58,17 @@ impl GatewaySession { } } + #[allow(dead_code)] fn kind(&self) -> SessionKind { self.kind } + #[allow(dead_code)] fn os_session(&self) -> &Session { &self.session } + #[allow(dead_code)] fn is_host_ready(&self) -> bool { self.is_host_ready } @@ -99,15 +102,15 @@ impl SessionManagerCtx { } } -pub struct SessionManager { +pub(crate) struct SessionManager { ctx: RwLock, - service_event_tx: mpsc::UnboundedSender, - service_event_rx: mpsc::UnboundedReceiver, + service_event_tx: mpsc::Sender, + service_event_rx: mpsc::Receiver, } impl Default for SessionManager { fn default() -> Self { - let (service_event_tx, service_event_rx) = mpsc::unbounded_channel(); + let (service_event_tx, service_event_rx) = mpsc::channel(100); Self { ctx: RwLock::new(SessionManagerCtx::default()), @@ -118,7 +121,7 @@ impl Default for SessionManager { } impl SessionManager { - pub(crate) fn service_event_tx(&self) -> mpsc::UnboundedSender { + pub(crate) fn service_event_tx(&self) -> mpsc::Sender { self.service_event_tx.clone() } } @@ -147,7 +150,7 @@ impl Task for SessionManager { event } else { error!("Service event channel closed"); - // Channel closed, all senders were dropped + // Channel closed, all senders were dropped. break; }; @@ -162,44 +165,44 @@ impl Task for SessionManager { } AgentServiceEvent::SessionDisconnect(id) => { info!(%id, "Session disconnected"); - try_terminate_host_process(&id); + terminate_host_process(&id); ctx.write().await.unregister_session(&id); } AgentServiceEvent::SessionRemoteConnect(id) => { info!(%id, "Remote session connected"); - // Terminate old host process if it is already running - try_terminate_host_process(&id); + // Terminate old host process if it is already running. + terminate_host_process(&id); { let mut ctx = ctx.write().await; ctx.register_session(&id, SessionKind::Remote); - try_start_host_process(&mut ctx, &id)?; + start_host_process_if_not_running(&mut ctx, &id)?; } } AgentServiceEvent::SessionRemoteDisconnect(id) => { info!(%id, "Remote session disconnected"); // Terminate host process when remote session is disconnected // (NOTE: depending on the system settings, session could - // still be running in the background after RDP disconnect) - try_terminate_host_process(&id); + // still be running in the background after RDP disconnect). + terminate_host_process(&id); ctx.write().await.unregister_session(&id); } AgentServiceEvent::SessionLogon(id) => { info!(%id, "Session logged on"); - // Terminate old host process if it is already running - try_terminate_host_process(&id); + // Terminate old host process if it is already running. + terminate_host_process(&id); // NOTE: In some cases, SessionRemoteConnect is fired before // an actual user is logged in, therefore we need to try start the host - // app on logon, if not yet started + // app on logon, if not yet started. let mut ctx = ctx.write().await; - try_start_host_process(&mut ctx, &id)?; + start_host_process_if_not_running(&mut ctx, &id)?; } AgentServiceEvent::SessionLogoff(id) => { info!(%id, "Session logged off"); - if let Some(mut session) = ctx.write().await.get_session_mut(&id) { + if let Some(session) = ctx.write().await.get_session_mut(&id) { // When a user logs off, host process will be stopped by the system; // Console sessions could be reused for different users, therefore // we should not remove the session from the list, but mark it as @@ -224,7 +227,7 @@ impl Task for SessionManager { } /// Starts Devolutions Host process in the target session. -fn try_start_host_process(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { +fn start_host_process_if_not_running(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { match ctx.get_session_mut(session) { Some(gw_session) => { if is_host_running_in_session(session)? { @@ -239,8 +242,8 @@ fn try_start_host_process(ctx: &mut SessionManagerCtx, session: &Session) -> any info!(%session, "Host process started in session"); gw_session.set_host_ready(true); } - Err(err) => { - error!(%err, %session, "Failed to start host process for session"); + Err(error) => { + error!(%error, %session, "Failed to start host process for session"); } } } @@ -253,7 +256,7 @@ fn try_start_host_process(ctx: &mut SessionManagerCtx, session: &Session) -> any } /// Terminates Devolutions Host process in the target session. -fn try_terminate_host_process(session: &Session) { +fn terminate_host_process(session: &Session) { match terminate_process_by_name_in_session(HOST_BINARY, session.id) { Ok(false) => { trace!(%session, "Host process is not running in the session"); @@ -261,8 +264,8 @@ fn try_terminate_host_process(session: &Session) { Ok(true) => { info!(%session, "Host process terminated in session"); } - Err(err) => { - error!(%err, %session, "Failed to terminate host process in session"); + Err(error) => { + error!(%error, %session, "Failed to terminate host process in session"); } } } @@ -285,7 +288,7 @@ fn start_host_process(session: &Session) -> anyhow::Result<()> { let mut startup_info = StartupInfo::default(); // Run with GUI access - // NOTE: silent clippy warning, just to be more explicit about `show_window` value + // NOTE: silent clippy warning, just to be more explicit about `show_window` value. #[allow(clippy::field_reassign_with_default)] { startup_info.show_window = u16::try_from(SW_SHOW.0).expect("BUG: SW_SHOW always fit u16"); @@ -311,9 +314,9 @@ fn start_host_process(session: &Session) -> anyhow::Result<()> { info!("{HOST_BINARY} started in session {session}"); Ok(()) } - Err(err) => { - error!(%err, "Failed to start {HOST_BINARY} in session {session}"); - Err(err) + Err(error) => { + error!(%error, "Failed to start {HOST_BINARY} in session {session}"); + Err(error) } } } diff --git a/devolutions-agent/src/updater/io.rs b/devolutions-agent/src/updater/io.rs index a11f6afd3..14e8bc369 100644 --- a/devolutions-agent/src/updater/io.rs +++ b/devolutions-agent/src/updater/io.rs @@ -62,8 +62,8 @@ pub fn remove_file_on_reboot_impl(file_path: &Utf8Path) -> Result<(), UpdaterErr let move_result = unsafe { MoveFileExW(&hstring_file_path, None, MOVEFILE_DELAY_UNTIL_REBOOT) }; - if let Err(err) = move_result { - warn!(%err, %file_path, "Failed to mark file for deletion on reboot"); + if let Err(error) = move_result { + warn!(%error, %file_path, "Failed to mark file for deletion on reboot"); } Ok(()) diff --git a/devolutions-agent/src/updater/mod.rs b/devolutions-agent/src/updater/mod.rs index fc29493b5..5530dd555 100644 --- a/devolutions-agent/src/updater/mod.rs +++ b/devolutions-agent/src/updater/mod.rs @@ -78,8 +78,8 @@ impl Task for UpdaterTask { Ok(_) => { let _ = file_change_tx.notify_waiters(); } - Err(err) => { - error!(%err, "Failed to watch update.json file"); + Err(error) => { + error!(%error, "Failed to watch update.json file"); } }) .context("failed to create file notify debouncer")?; @@ -100,8 +100,8 @@ impl Task for UpdaterTask { let update_json = match read_update_json(&update_file_path).await { Ok(update_json) => update_json, - Err(err) => { - error!(%err, "Failed to parse `update.json`"); + Err(error) => { + error!(%error, "Failed to parse `update.json`"); // Allow this error to be non-critical, as this file could be // updated later to be valid again continue; @@ -113,8 +113,8 @@ impl Task for UpdaterTask { for product in PRODUCTS { let update_order = match check_for_updates(*product, &update_json).await { Ok(order) => order, - Err(err) => { - error!(%product, %err, "Failed to check for updates for a product."); + Err(error) => { + error!(%product, %error, "Failed to check for updates for a product."); continue; } }; @@ -129,8 +129,8 @@ impl Task for UpdaterTask { } for (product, order) in update_orders { - if let Err(err) = update_product(conf.clone(), product, order).await { - error!(%product, %err, "Failed to update product"); + if let Err(error) = update_product(conf.clone(), product, order).await { + error!(%product, %error, "Failed to update product"); } } } @@ -271,8 +271,9 @@ async fn init_update_json() -> anyhow::Result { } Err(err) => { // Remove update.json file if failed to set permissions - std::fs::remove_file(update_file_path.as_std_path()) - .unwrap_or_else(|err| warn!(%err, "Failed to remove update.json file after failed permissions set")); + std::fs::remove_file(update_file_path.as_std_path()).unwrap_or_else( + |error| warn!(%error, "Failed to remove update.json file after failed permissions set"), + ); // Treat as fatal error return Err(anyhow!(err).context("failed to set update.json file permissions")); diff --git a/devolutions-agent/src/updater/package.rs b/devolutions-agent/src/updater/package.rs index c5c08188d..7509eb20e 100644 --- a/devolutions-agent/src/updater/package.rs +++ b/devolutions-agent/src/updater/package.rs @@ -41,8 +41,8 @@ async fn install_msi(ctx: &UpdaterCtx, path: &Utf8Path) -> Result<(), UpdaterErr info!("MSI installation log: {log_path}"); // Schedule log file for deletion on reboot - if let Err(err) = remove_file_on_reboot(&log_path) { - error!(%err, "Failed to schedule log file for deletion on reboot"); + if let Err(error) = remove_file_on_reboot(&log_path) { + error!(%error, "Failed to schedule log file for deletion on reboot"); } } diff --git a/devolutions-host/Cargo.toml b/devolutions-host/Cargo.toml index 3101d282b..e6eb2c5b8 100644 --- a/devolutions-host/Cargo.toml +++ b/devolutions-host/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "devolutions-host" -version = "2024.3.1" +version = "2024.3.2" edition = "2021" license = "MIT/Apache-2.0" authors = ["Devolutions Inc. "] diff --git a/devolutions-host/src/dvc.rs b/devolutions-host/src/dvc.rs index 695a70823..f7b8b2298 100644 --- a/devolutions-host/src/dvc.rs +++ b/devolutions-host/src/dvc.rs @@ -28,12 +28,12 @@ pub fn loop_dvc(config: ConfHandle) { Ok(h_file) => { info!("Virtual channel opened"); - if let Err(err) = handle_virtual_channel(h_file) { - error!(%err, "DVC handling falied"); + if let Err(error) = handle_virtual_channel(h_file) { + error!(%error, "DVC handling falied"); } } - Err(err) => { - error!(%err, "Failed to open virtual channel"); + Err(error) => { + error!(%error, "Failed to open virtual channel"); // NOTE: Not exiting the program here, as it is not the main functionality } } diff --git a/devolutions-host/src/main.rs b/devolutions-host/src/main.rs index 648394b3e..d506fc07d 100644 --- a/devolutions-host/src/main.rs +++ b/devolutions-host/src/main.rs @@ -15,7 +15,7 @@ use anyhow::Context; use std::sync::mpsc; fn main() -> anyhow::Result<()> { - // Ensure per-user data dir exists + // Ensure per-user data dir exists. std::fs::create_dir_all(get_data_dir()).context("Failed to create data directory")?; @@ -25,7 +25,7 @@ fn main() -> anyhow::Result<()> { info!("Starting Devolutions Host"); - // TMP: Copy-paste from MSRDPEX project for testing purposes + // TMP: Copy-paste from MSRDPEX project for testing purposes. #[cfg(windows)] loop_dvc(config); diff --git a/tools/bump-version.ps1 b/tools/bump-version.ps1 index a2953767c..736d2a240 100755 --- a/tools/bump-version.ps1 +++ b/tools/bump-version.ps1 @@ -15,6 +15,7 @@ $targetFiles = @( './jetsocat/Cargo.toml' './devolutions-gateway/Cargo.toml' './devolutions-agent/Cargo.toml' + './devolutions-host/Cargo.toml' './powershell/DevolutionsGateway/DevolutionsGateway.psd1' './Cargo.lock' './fuzz/Cargo.lock' @@ -42,4 +43,3 @@ catch { finally { Pop-Location } - From a6c808993aadbf0ccc7638c77fe2ff47a2b096b5 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 3 Sep 2024 13:53:11 +0300 Subject: [PATCH 06/10] refactoring: more refactoring --- crates/win-api-wrappers/src/process.rs | 1 + .../src/{session_manager/mod.rs => session_manager.rs} | 0 2 files changed, 1 insertion(+) rename devolutions-agent/src/{session_manager/mod.rs => session_manager.rs} (100%) diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 709a8a47b..239c37bdf 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -870,6 +870,7 @@ fn terminate_process_by_name_impl(process_name: &str, session_id: Option) - } if str::eq_ignore_ascii_case(process.executable_name()?.as_str(), process_name) { + // SAFETY: `OpenProcess` is always safe to call and returns a valid handle on success. let process = unsafe { OpenProcess(PROCESS_TERMINATE, false, process.process_id()) }; match process { diff --git a/devolutions-agent/src/session_manager/mod.rs b/devolutions-agent/src/session_manager.rs similarity index 100% rename from devolutions-agent/src/session_manager/mod.rs rename to devolutions-agent/src/session_manager.rs From c2eea776f0eac1b9b4c0ea4e17240843dd40fb3b Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Wed, 4 Sep 2024 17:45:28 +0300 Subject: [PATCH 07/10] feat(host): devolutions-host installer changes (#1005) --- .github/workflows/ci.yml | 12 +++++++++--- .github/workflows/package.yml | 5 +++-- ci/tlk.ps1 | 8 ++++++++ .../Actions/AgentActions.cs | 16 +++++++++++++++- .../Actions/CustomActions.cs | 19 +++++++++++++++---- package/AgentWindowsManaged/Program.cs | 5 ++++- .../AgentWindowsManaged/Resources/Includes.cs | 2 ++ 7 files changed, 56 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f73abbaa7..d1e242d40 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -537,9 +537,14 @@ jobs: $DAgentPedmShellExtMsix = Join-Path $TargetOutputPath "DevolutionsPedmShellExt.msix" echo "dagent-pedm-shell-ext-msix=$DAgentPedmShellExtMsix" >> $Env:GITHUB_OUTPUT - + $DAgentPedmHook = Join-Path $TargetOutputPath "devolutions_pedm_hook.dll" echo "dagent-pedm-hook=$DAgentPedmHook" >> $Env:GITHUB_OUTPUT + + $DAgentDevolutionsHostExecutable = Join-Path $TargetOutputPath "DevolutionsHost.exe" + echo "dagent-devolutions-host-executable=$DAgentDevolutionsHostExecutable" >> $Env:GITHUB_OUTPUT + + } $DAgentExecutable = Join-Path $TargetOutputPath $ExecutableFileName @@ -603,8 +608,9 @@ jobs: $Env:DAGENT_PEDM_HOOK = "${{ steps.load-variables.outputs.dagent-pedm-hook }}" $Env:DAGENT_PEDM_SHELL_EXT_DLL = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-dll }}" $Env:DAGENT_PEDM_SHELL_EXT_MSIX = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-msix }}" + $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-devolutions-host-executable }}" } - + ./ci/tlk.ps1 build -Product agent -Platform ${{ matrix.os }} -Architecture ${{ matrix.arch }} -CargoProfile ${{ needs.preflight.outputs.rust-profile }} - name: Package @@ -620,6 +626,7 @@ jobs: $Env:DAGENT_PEDM_HOOK = "${{ steps.load-variables.outputs.dagent-pedm-hook }}" $Env:DAGENT_PEDM_SHELL_EXT_DLL = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-dll }}" $Env:DAGENT_PEDM_SHELL_EXT_MSIX = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-msix }}" + $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-devolutions-host-executable }}" } ./ci/tlk.ps1 package -Product agent -Platform ${{ matrix.os }} -Architecture ${{ matrix.arch }} -CargoProfile ${{ needs.preflight.outputs.rust-profile }} @@ -827,4 +834,3 @@ jobs: destination_path: /Gateway/${{ steps.prepare.outputs.version }}-${{ steps.timestamp.outputs.timestamp }}-${{ steps.prepare.outputs.short-ref }} remote: prereleases source_path: ${{ steps.prepare.outputs.files-to-upload }} - diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 6133eeab7..a5e9aff74 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -251,7 +251,7 @@ jobs: $appx.Save($xmlTextWriter) $xmlTextWriter.Close() & 'MakeAppx.exe' pack /d $UnpackedMsix /p $PackedMsix /nv - + Remove-Item $UnpackedMsix -Recurse -Force | Out-Null } @@ -326,6 +326,7 @@ jobs: $Env:DAGENT_PEDM_SHELL_EXT_DLL = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsPedmShellExt.dll' | Select -First 1 $Env:DAGENT_PEDM_SHELL_EXT_MSIX = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsPedmShellExt.msix' | Select -First 1 $Env:DAGENT_PEDM_HOOK = Get-ChildItem -Path $PackageRoot -Recurse -Include 'devolutions_pedm_hook.dll' | Select -First 1 + $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsHost.dll' | Select -First 1 ./ci/tlk.ps1 package -Product agent -PackageOption generate @@ -431,7 +432,7 @@ jobs: shell: pwsh run: | $PackageRoot = Join-Path ${{ runner.temp }} ${{ matrix.project}} - + Get-ChildItem -Path $PackageRoot -Recurse Compress-Archive "$PackageRoot\windows\x86_64\*.pdb" "$PackageRoot\windows\x86_64\DevolutionsAgent-x86_64-${{ needs.preflight.outputs.version }}.symbols.zip" -CompressionLevel Optimal diff --git a/ci/tlk.ps1 b/ci/tlk.ps1 index 0880476cd..e223b54b2 100644 --- a/ci/tlk.ps1 +++ b/ci/tlk.ps1 @@ -432,6 +432,8 @@ class TlkRecipe $Env:DAGENT_PEDM_HOOK } elseif ($CargoPackage.Name -Eq "devolutions-pedm-shell-ext" -And (Test-Path Env:DAGENT_PEDM_SHELL_EXT_DLL)) { $Env:DAGENT_PEDM_SHELL_EXT_DLL + } elseif ($CargoPackage.Name -Eq "devolutions-host" -And (Test-Path Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE)) { + $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE } else { $null } @@ -500,6 +502,12 @@ class TlkRecipe Copy-Item -Path $builtDesktopPdb -Destination $(Get-DestinationSymbolFile $Env:DAGENT_PEDM_DESKTOP_EXECUTABLE $this.Target) } + + if (Test-Path Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE) { + $hostExe = Get-ChildItem -Recurse -Include 'devolutions-host.exe' | Select-Object -First 1 + + Copy-Item -Path $hostExe -Destination $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE + } } Pop-Location diff --git a/package/AgentWindowsManaged/Actions/AgentActions.cs b/package/AgentWindowsManaged/Actions/AgentActions.cs index 10e7c0426..8108a6ae9 100644 --- a/package/AgentWindowsManaged/Actions/AgentActions.cs +++ b/package/AgentWindowsManaged/Actions/AgentActions.cs @@ -120,7 +120,7 @@ internal static class AgentActions Impersonate = false, UsesProperties = UseProperties(new [] { AgentProperties.installId }) }; - + private static readonly ElevatedManagedAction cleanAgentConfigIfNeededRollback = new( new Id($"CA.{nameof(cleanAgentConfigIfNeededRollback)}"), CustomActions.CleanAgentConfigRollback, @@ -252,6 +252,19 @@ internal static class AgentActions Condition = Includes.PEDM_FEATURE.BeingUninstall(), }; + private static readonly ElevatedManagedAction installHost = new( + CustomActions.InstallHost + ) + { + Id = new Id("installHost"), + Feature = Includes.PEDM_FEATURE, + Sequence = Sequence.InstallExecuteSequence, + Return = Return.check, + Step = Step.InstallFiles, + When = When.After, + Condition = Includes.PEDM_FEATURE.BeingInstall(), + }; + private static string UseProperties(IEnumerable properties) { if (!properties?.Any() ?? false) @@ -285,6 +298,7 @@ private static string UseProperties(IEnumerable properties) cleanupPedmShellExt, uninstallPedmShellExt, installPedmShellExt, + installHost, cleanAgentConfigIfNeeded, cleanAgentConfigIfNeededRollback, diff --git a/package/AgentWindowsManaged/Actions/CustomActions.cs b/package/AgentWindowsManaged/Actions/CustomActions.cs index e96c35b77..6d934ca6e 100644 --- a/package/AgentWindowsManaged/Actions/CustomActions.cs +++ b/package/AgentWindowsManaged/Actions/CustomActions.cs @@ -197,8 +197,7 @@ public static ActionResult GetInstalledNetFx45Version(Session session) return ActionResult.Success; } - [CustomAction] - public static ActionResult InstallPedm(Session session) + static ActionResult EnableAgentFeature(Session session, string feature) { string path = Path.Combine(ProgramDataDirectory, "agent.json"); @@ -215,7 +214,7 @@ public static ActionResult InstallPedm(Session session) // ignored. Previous config is either invalid or non existent. } - config["Pedm"] = new Dictionary { { "Enabled", true } }; + config[feature] = new Dictionary { { "Enabled", true } }; using var writer = new StreamWriter(path); writer.Write(JsonConvert.SerializeObject(config)); @@ -224,11 +223,23 @@ public static ActionResult InstallPedm(Session session) } catch (Exception e) { - session.Log($"failed to install pedm: {e}"); + session.Log($"failed to install {feature}: {e}"); return ActionResult.Failure; } } + [CustomAction] + public static ActionResult InstallPedm(Session session) + { + return EnableAgentFeature(session, "Pedm"); + } + + [CustomAction] + public static ActionResult InstallHost(Session session) + { + return EnableAgentFeature(session, "SessionHost"); + } + [CustomAction] public static ActionResult RestartAgent(Session session) { diff --git a/package/AgentWindowsManaged/Program.cs b/package/AgentWindowsManaged/Program.cs index ca4d66427..3f373f7ac 100644 --- a/package/AgentWindowsManaged/Program.cs +++ b/package/AgentWindowsManaged/Program.cs @@ -119,6 +119,8 @@ private static string ResolveArtifact(string varName, string error) private static string DevolutionsPedmShellExtMsix => ResolveArtifact("DAGENT_PEDM_SHELL_EXT_MSIX", "The PEDM shell extension MSIX was not found"); + private static string DevolutionsHost => ResolveArtifact("DAGENT_DEVOLUTIONS_HOST_EXECUTABLE", "The Devolutions Host executable was not found"); + private static Version DevolutionsAgentVersion { get @@ -281,7 +283,8 @@ static void Main() }, new (Includes.PEDM_FEATURE, DevolutionsPedmHook), new (Includes.PEDM_FEATURE, DevolutionsPedmShellExtDll), - new (Includes.PEDM_FEATURE, DevolutionsPedmShellExtMsix) + new (Includes.PEDM_FEATURE, DevolutionsPedmShellExtMsix), + new (Includes.HOST_FEATURE, DevolutionsHost) }, Dirs = new[] { diff --git a/package/AgentWindowsManaged/Resources/Includes.cs b/package/AgentWindowsManaged/Resources/Includes.cs index dccf16150..8ef163a10 100644 --- a/package/AgentWindowsManaged/Resources/Includes.cs +++ b/package/AgentWindowsManaged/Resources/Includes.cs @@ -28,6 +28,8 @@ internal static class Includes internal static Feature PEDM_FEATURE = new Feature("Devolutions PEDM", "Installs Devolutions PEDM", false); + internal static Feature HOST_FEATURE = new Feature("Devolutions Host", "Installs Devolutions Host", false); + /// /// SDDL string representing desired %programdata%\devolutions\agent ACL /// Easiest way to generate an SDDL is to configure the required access, and then query the path with PowerShell: `Get-Acl | Format-List` From 84b971a1a02c9dd340a80c9224e396fe60c43905 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Mon, 9 Sep 2024 11:54:57 +0300 Subject: [PATCH 08/10] refactor: rename Devolutions Host to Devolutions Session --- .github/workflows/ci.yml | 10 +- .github/workflows/package.yml | 2 +- Cargo.lock | 36 +++---- Cargo.toml | 4 +- ci/tlk.ps1 | 12 +-- devolutions-agent/src/config.rs | 16 +-- devolutions-agent/src/service.rs | 2 +- devolutions-agent/src/session_manager.rs | 100 +++++++++--------- .../Cargo.toml | 4 +- .../build.rs | 2 +- .../src/config.rs | 16 +-- .../src/dvc.rs | 0 .../src/lib.rs | 0 .../src/log.rs | 8 +- .../src/main.rs | 8 +- package.ps1 | 40 +++++++ .../Actions/AgentActions.cs | 12 +-- .../Actions/CustomActions.cs | 12 +-- package/AgentWindowsManaged/Program.cs | 4 +- .../AgentWindowsManaged/Resources/Includes.cs | 2 +- tools/bump-version.ps1 | 2 +- 21 files changed, 165 insertions(+), 127 deletions(-) rename {devolutions-host => devolutions-session}/Cargo.toml (88%) rename {devolutions-host => devolutions-session}/build.rs (98%) rename {devolutions-host => devolutions-session}/src/config.rs (94%) rename {devolutions-host => devolutions-session}/src/dvc.rs (100%) rename {devolutions-host => devolutions-session}/src/lib.rs (100%) rename {devolutions-host => devolutions-session}/src/log.rs (73%) rename {devolutions-host => devolutions-session}/src/main.rs (85%) create mode 100644 package.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1e242d40..e4eea07fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -541,10 +541,8 @@ jobs: $DAgentPedmHook = Join-Path $TargetOutputPath "devolutions_pedm_hook.dll" echo "dagent-pedm-hook=$DAgentPedmHook" >> $Env:GITHUB_OUTPUT - $DAgentDevolutionsHostExecutable = Join-Path $TargetOutputPath "DevolutionsHost.exe" - echo "dagent-devolutions-host-executable=$DAgentDevolutionsHostExecutable" >> $Env:GITHUB_OUTPUT - - + $DAgentSessionExecutable = Join-Path $TargetOutputPath "DevolutionsSession.exe" + echo "dagent-session-executable=$DAgentSessionExecutable" >> $Env:GITHUB_OUTPUT } $DAgentExecutable = Join-Path $TargetOutputPath $ExecutableFileName @@ -608,7 +606,7 @@ jobs: $Env:DAGENT_PEDM_HOOK = "${{ steps.load-variables.outputs.dagent-pedm-hook }}" $Env:DAGENT_PEDM_SHELL_EXT_DLL = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-dll }}" $Env:DAGENT_PEDM_SHELL_EXT_MSIX = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-msix }}" - $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-devolutions-host-executable }}" + $Env:DAGENT_SESSION_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-session-executable }}" } ./ci/tlk.ps1 build -Product agent -Platform ${{ matrix.os }} -Architecture ${{ matrix.arch }} -CargoProfile ${{ needs.preflight.outputs.rust-profile }} @@ -626,7 +624,7 @@ jobs: $Env:DAGENT_PEDM_HOOK = "${{ steps.load-variables.outputs.dagent-pedm-hook }}" $Env:DAGENT_PEDM_SHELL_EXT_DLL = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-dll }}" $Env:DAGENT_PEDM_SHELL_EXT_MSIX = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-msix }}" - $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-devolutions-host-executable }}" + $Env:DAGENT_SESSION_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-session-executable }}" } ./ci/tlk.ps1 package -Product agent -Platform ${{ matrix.os }} -Architecture ${{ matrix.arch }} -CargoProfile ${{ needs.preflight.outputs.rust-profile }} diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index a5e9aff74..8c31c67b9 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -326,7 +326,7 @@ jobs: $Env:DAGENT_PEDM_SHELL_EXT_DLL = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsPedmShellExt.dll' | Select -First 1 $Env:DAGENT_PEDM_SHELL_EXT_MSIX = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsPedmShellExt.msix' | Select -First 1 $Env:DAGENT_PEDM_HOOK = Get-ChildItem -Path $PackageRoot -Recurse -Include 'devolutions_pedm_hook.dll' | Select -First 1 - $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsHost.dll' | Select -First 1 + $Env:DAGENT_SESSION_EXECUTABLE = Get-ChildItem -Path $PackageRoot -Recurse -Include 'DevolutionsSession.exe' | Select -First 1 ./ci/tlk.ps1 package -Product agent -PackageOption generate diff --git a/Cargo.lock b/Cargo.lock index f050ec12a..133118f4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1118,24 +1118,6 @@ dependencies = [ "tokio 1.38.1", ] -[[package]] -name = "devolutions-host" -version = "2024.3.2" -dependencies = [ - "anyhow", - "camino", - "cfg-if", - "ctrlc", - "devolutions-log", - "embed-resource", - "parking_lot", - "serde", - "serde_json", - "tap", - "tracing", - "windows 0.58.0", -] - [[package]] name = "devolutions-log" version = "0.0.0" @@ -1242,6 +1224,24 @@ dependencies = [ "windows-core 0.58.0", ] +[[package]] +name = "devolutions-session" +version = "2024.3.2" +dependencies = [ + "anyhow", + "camino", + "cfg-if", + "ctrlc", + "devolutions-log", + "embed-resource", + "parking_lot", + "serde", + "serde_json", + "tap", + "tracing", + "windows 0.58.0", +] + [[package]] name = "digest" version = "0.10.7" diff --git a/Cargo.toml b/Cargo.toml index db39caac2..b65d98cda 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,14 +4,14 @@ members = [ "crates/*", "devolutions-agent", "devolutions-gateway", - "devolutions-host", + "devolutions-session", "jetsocat", "tools/generate-openapi", ] default-members = [ "devolutions-agent", "devolutions-gateway", - "devolutions-host", + "devolutions-session", "jetsocat", ] diff --git a/ci/tlk.ps1 b/ci/tlk.ps1 index e223b54b2..7d3b507d5 100644 --- a/ci/tlk.ps1 +++ b/ci/tlk.ps1 @@ -345,7 +345,7 @@ class TlkRecipe if ($this.Target.IsWindows()) { $agentPackages += [TlkPackage]::new("devolutions-pedm-hook", "crates/devolutions-pedm-hook", $true) $agentPackages += [TlkPackage]::new("devolutions-pedm-shell-ext", "crates/devolutions-pedm-shell-ext", $true) - $agentPackages += [TlkPackage]::new("devolutions-host", "devolutions-host", $false) + $agentPackages += [TlkPackage]::new("devolutions-session", "devolutions-session", $false) } $agentPackages @@ -432,8 +432,8 @@ class TlkRecipe $Env:DAGENT_PEDM_HOOK } elseif ($CargoPackage.Name -Eq "devolutions-pedm-shell-ext" -And (Test-Path Env:DAGENT_PEDM_SHELL_EXT_DLL)) { $Env:DAGENT_PEDM_SHELL_EXT_DLL - } elseif ($CargoPackage.Name -Eq "devolutions-host" -And (Test-Path Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE)) { - $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE + } elseif ($CargoPackage.Name -Eq "devolutions-session" -And (Test-Path Env:DAGENT_SESSION_EXECUTABLE)) { + $Env:DAGENT_SESSION_EXECUTABLE } else { $null } @@ -503,10 +503,10 @@ class TlkRecipe } - if (Test-Path Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE) { - $hostExe = Get-ChildItem -Recurse -Include 'devolutions-host.exe' | Select-Object -First 1 + if (Test-Path Env:DAGENT_SESSION_EXECUTABLE) { + $hostExe = Get-ChildItem -Recurse -Include 'devolutions-session.exe' | Select-Object -First 1 - Copy-Item -Path $hostExe -Destination $Env:DAGENT_DEVOLUTIONS_HOST_EXECUTABLE + Copy-Item -Path $hostExe -Destination $Env:DAGENT_SESSION_EXECUTABLE } } diff --git a/devolutions-agent/src/config.rs b/devolutions-agent/src/config.rs index 0385bd78c..1e6dc64a7 100644 --- a/devolutions-agent/src/config.rs +++ b/devolutions-agent/src/config.rs @@ -18,7 +18,7 @@ pub struct Conf { pub updater: dto::UpdaterConf, pub remote_desktop: RemoteDesktopConf, pub pedm: dto::PedmConf, - pub session_host: dto::SessionHostConf, + pub session: dto::SessionConf, pub debug: dto::DebugConf, } @@ -45,7 +45,7 @@ impl Conf { updater: conf_file.updater.clone().unwrap_or_default(), remote_desktop, pedm: conf_file.pedm.clone().unwrap_or_default(), - session_host: conf_file.session_host.clone().unwrap_or_default(), + session: conf_file.session.clone().unwrap_or_default(), debug: conf_file.debug.clone().unwrap_or_default(), }) } @@ -257,13 +257,13 @@ pub mod dto { #[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "PascalCase")] - pub struct SessionHostConf { - /// Enable Session host module (disabled by default) + pub struct SessionConf { + /// Enable Devolutions Session module (disabled by default) pub enabled: bool, } #[allow(clippy::derivable_impls)] // Just to be explicit about the default values of the config. - impl Default for SessionHostConf { + impl Default for SessionConf { fn default() -> Self { Self { enabled: false } } @@ -296,9 +296,9 @@ pub mod dto { #[serde(default, skip_serializing_if = "Option::is_none")] pub pedm: Option, - /// Devolutions Session Host configuration + /// Devolutions Session configuration #[serde(default, skip_serializing_if = "Option::is_none")] - pub session_host: Option, + pub session: Option, /// (Unstable) Unsafe debug options for developers #[serde(rename = "__debug__", skip_serializing_if = "Option::is_none")] @@ -320,7 +320,7 @@ pub mod dto { remote_desktop: None, pedm: None, debug: None, - session_host: Some(SessionHostConf { enabled: false }), + session: Some(SessionConf { enabled: false }), rest: serde_json::Map::new(), } } diff --git a/devolutions-agent/src/service.rs b/devolutions-agent/src/service.rs index 309bbc9e3..f7c44ccdf 100644 --- a/devolutions-agent/src/service.rs +++ b/devolutions-agent/src/service.rs @@ -213,7 +213,7 @@ async fn spawn_tasks(conf_handle: ConfHandle) -> anyhow::Result { tasks.register(PedmTask::new()) } - if conf.session_host.enabled { + if conf.session.enabled { let session_manager = SessionManager::default(); let tx = session_manager.service_event_tx(); tasks.register(session_manager); diff --git a/devolutions-agent/src/session_manager.rs b/devolutions-agent/src/session_manager.rs index b308c59af..801dc85da 100644 --- a/devolutions-agent/src/session_manager.rs +++ b/devolutions-agent/src/session_manager.rs @@ -1,4 +1,4 @@ -//! Module for starting and managing the Devolutions Host process in user sessions. +//! Module for starting and managing the Devolutions Session process in user sessions. use tokio::sync::{mpsc, RwLock}; @@ -20,7 +20,7 @@ use windows::Win32::System::Threading::{ }; use windows::Win32::UI::WindowsAndMessaging::SW_SHOW; -const HOST_BINARY: &str = "DevolutionsHost.exe"; +const SESSION_BINARY: &str = "DevolutionsSession.exe"; #[derive(Debug, Clone, Copy)] pub(crate) enum SessionKind { @@ -35,7 +35,7 @@ pub(crate) enum SessionKind { struct GatewaySession { session: Session, kind: SessionKind, - is_host_ready: bool, + is_session_ready: bool, } // NOTE: `ceviche::controller::Session` do not implement `Debug` for session. @@ -44,7 +44,7 @@ impl Debug for GatewaySession { f.debug_struct("GatewaySession") .field("session", &self.session.id) .field("kind", &self.kind) - .field("is_host_ready", &self.is_host_ready) + .field("is_session_ready", &self.is_session_ready) .finish() } } @@ -54,7 +54,7 @@ impl GatewaySession { Self { session, kind, - is_host_ready: false, + is_session_ready: false, } } @@ -69,12 +69,12 @@ impl GatewaySession { } #[allow(dead_code)] - fn is_host_ready(&self) -> bool { - self.is_host_ready + fn is_session_ready(&self) -> bool { + self.is_session_ready } - fn set_host_ready(&mut self, is_ready: bool) { - self.is_host_ready = is_ready; + fn set_session_ready(&mut self, is_ready: bool) { + self.is_session_ready = is_ready; } } @@ -159,55 +159,55 @@ impl Task for SessionManager { info!(%id, "Session connected"); let mut ctx = ctx.write().await; ctx.register_session(&id, SessionKind::Console); - // We only start the host process for remote sessions (initiated - // via RDP), as Host process with DVC handler is only needed for remote + // We only start the session process for remote sessions (initiated + // via RDP), as session process with DVC handler is only needed for remote // sessions. } AgentServiceEvent::SessionDisconnect(id) => { info!(%id, "Session disconnected"); - terminate_host_process(&id); + terminate_session_process(&id); ctx.write().await.unregister_session(&id); } AgentServiceEvent::SessionRemoteConnect(id) => { info!(%id, "Remote session connected"); - // Terminate old host process if it is already running. - terminate_host_process(&id); + // Terminate old session process if it is already running. + terminate_session_process(&id); { let mut ctx = ctx.write().await; ctx.register_session(&id, SessionKind::Remote); - start_host_process_if_not_running(&mut ctx, &id)?; + start_session_process_if_not_running(&mut ctx, &id)?; } } AgentServiceEvent::SessionRemoteDisconnect(id) => { info!(%id, "Remote session disconnected"); - // Terminate host process when remote session is disconnected + // Terminate session process when remote session is disconnected // (NOTE: depending on the system settings, session could // still be running in the background after RDP disconnect). - terminate_host_process(&id); + terminate_session_process(&id); ctx.write().await.unregister_session(&id); } AgentServiceEvent::SessionLogon(id) => { info!(%id, "Session logged on"); - // Terminate old host process if it is already running. - terminate_host_process(&id); + // Terminate old session process if it is already running. + terminate_session_process(&id); // NOTE: In some cases, SessionRemoteConnect is fired before - // an actual user is logged in, therefore we need to try start the host - // app on logon, if not yet started. + // an actual user is logged in, therefore we need to try start the + // session app on logon, if not yet started. let mut ctx = ctx.write().await; - start_host_process_if_not_running(&mut ctx, &id)?; + start_session_process_if_not_running(&mut ctx, &id)?; } AgentServiceEvent::SessionLogoff(id) => { info!(%id, "Session logged off"); if let Some(session) = ctx.write().await.get_session_mut(&id) { - // When a user logs off, host process will be stopped by the system; + // When a user logs off, session process will be stopped by the system; // Console sessions could be reused for different users, therefore // we should not remove the session from the list, but mark it as - // not yet ready (host will be started as soon as new user logs in). - session.set_host_ready(false); + // not yet ready (session will be started as soon as new user logs in). + session.set_session_ready(false); } } _ => { @@ -226,24 +226,24 @@ impl Task for SessionManager { } } -/// Starts Devolutions Host process in the target session. -fn start_host_process_if_not_running(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { +/// Starts Devolutions Session process in the target session. +fn start_session_process_if_not_running(ctx: &mut SessionManagerCtx, session: &Session) -> anyhow::Result<()> { match ctx.get_session_mut(session) { Some(gw_session) => { - if is_host_running_in_session(session)? { - gw_session.set_host_ready(true); + if is_session_running_in_session(session)? { + gw_session.set_session_ready(true); return Ok(()); } - info!(%session, "Starting host process in session"); + info!(%session, "Starting session process in session"); - match start_host_process(session) { + match start_session_process(session) { Ok(()) => { - info!(%session, "Host process started in session"); - gw_session.set_host_ready(true); + info!(%session, "Session process started in session"); + gw_session.set_session_ready(true); } Err(error) => { - error!(%error, %session, "Failed to start host process for session"); + error!(%error, %session, "Failed to start session process for session"); } } } @@ -255,35 +255,35 @@ fn start_host_process_if_not_running(ctx: &mut SessionManagerCtx, session: &Sess Ok(()) } -/// Terminates Devolutions Host process in the target session. -fn terminate_host_process(session: &Session) { - match terminate_process_by_name_in_session(HOST_BINARY, session.id) { +/// Terminates Devolutions Session process in the target session. +fn terminate_session_process(session: &Session) { + match terminate_process_by_name_in_session(SESSION_BINARY, session.id) { Ok(false) => { - trace!(%session, "Host process is not running in the session"); + trace!(%session, "Session process is not running in the session"); } Ok(true) => { - info!(%session, "Host process terminated in session"); + info!(%session, "Session process terminated in session"); } Err(error) => { - error!(%error, %session, "Failed to terminate host process in session"); + error!(%error, %session, "Failed to terminate session process in session"); } } } -fn is_host_running_in_session(session: &Session) -> anyhow::Result { - let is_running = is_process_running_in_session(HOST_BINARY, session.id)?; +fn is_session_running_in_session(session: &Session) -> anyhow::Result { + let is_running = is_process_running_in_session(SESSION_BINARY, session.id)?; Ok(is_running) } -fn start_host_process(session: &Session) -> anyhow::Result<()> { +fn start_session_process(session: &Session) -> anyhow::Result<()> { if !session_has_logged_in_user(session.id)? { anyhow::bail!("Session {} does not have a logged in user", session); } - let host_app_path = host_app_path(); + let session_app_path = session_app_path(); let command_line = CommandLine::new(vec!["--session".to_owned(), session.to_string()]); - info!("Starting `{host_app_path}` in session `{session}`"); + info!("Starting `{session_app_path}` in session `{session}`"); let mut startup_info = StartupInfo::default(); @@ -298,7 +298,7 @@ fn start_host_process(session: &Session) -> anyhow::Result<()> { let start_result = create_process_in_session( session.id, - Some(host_app_path.as_std_path()), + Some(session_app_path.as_std_path()), Some(&command_line), None, None, @@ -311,22 +311,22 @@ fn start_host_process(session: &Session) -> anyhow::Result<()> { match start_result { Ok(_) => { - info!("{HOST_BINARY} started in session {session}"); + info!("{SESSION_BINARY} started in session {session}"); Ok(()) } Err(error) => { - error!(%error, "Failed to start {HOST_BINARY} in session {session}"); + error!(%error, "Failed to start {SESSION_BINARY} in session {session}"); Err(error) } } } -fn host_app_path() -> Utf8PathBuf { +fn session_app_path() -> Utf8PathBuf { let mut current_dir = Utf8PathBuf::from_path_buf(std::env::current_exe().expect("BUG: can't get current exe path")) .expect("BUG: OS should always return valid UTF-8 executable path"); current_dir.pop(); - current_dir.push(HOST_BINARY); + current_dir.push(SESSION_BINARY); current_dir } diff --git a/devolutions-host/Cargo.toml b/devolutions-session/Cargo.toml similarity index 88% rename from devolutions-host/Cargo.toml rename to devolutions-session/Cargo.toml index e6eb2c5b8..0d87c30bf 100644 --- a/devolutions-host/Cargo.toml +++ b/devolutions-session/Cargo.toml @@ -1,10 +1,10 @@ [package] -name = "devolutions-host" +name = "devolutions-session" version = "2024.3.2" edition = "2021" license = "MIT/Apache-2.0" authors = ["Devolutions Inc. "] -description = "Per-session host application for Devolutions Agent" +description = "Session host application for Devolutions Agent" build = "build.rs" publish = false diff --git a/devolutions-host/build.rs b/devolutions-session/build.rs similarity index 98% rename from devolutions-host/build.rs rename to devolutions-session/build.rs index 0b96bd32e..3fd609083 100644 --- a/devolutions-host/build.rs +++ b/devolutions-session/build.rs @@ -17,7 +17,7 @@ mod win { } fn generate_version_rc() -> String { - let output_name = "DevolutionsHost"; + let output_name = "DevolutionsSession"; let filename = format!("{}.exe", output_name); let company_name = "Devolutions Inc."; let legal_copyright = format!("Copyright 2020-2024 {}", company_name); diff --git a/devolutions-host/src/config.rs b/devolutions-session/src/config.rs similarity index 94% rename from devolutions-host/src/config.rs rename to devolutions-session/src/config.rs index f4dc37234..3e9623139 100644 --- a/devolutions-host/src/config.rs +++ b/devolutions-session/src/config.rs @@ -11,16 +11,16 @@ use std::sync::Arc; cfg_if! { if #[cfg(target_os = "windows")] { const COMPANY_DIR: &str = "Devolutions"; - const PROGRAM_DIR: &str = "Host"; - const APPLICATION_DIR: &str = "Devolutions\\Host"; + const PROGRAM_DIR: &str = "Session"; + const APPLICATION_DIR: &str = "Devolutions\\Session"; } else if #[cfg(target_os = "macos")] { const COMPANY_DIR: &str = "Devolutions"; - const PROGRAM_DIR: &str = "Host"; - const APPLICATION_DIR: &str = "Devolutions Host"; + const PROGRAM_DIR: &str = "Session"; + const APPLICATION_DIR: &str = "Devolutions Session"; } else { const COMPANY_DIR: &str = "devolutions"; - const PROGRAM_DIR: &str = "Host"; - const APPLICATION_DIR: &str = "devolutions-host"; + const PROGRAM_DIR: &str = "Session"; + const APPLICATION_DIR: &str = "devolutions-session"; } } @@ -38,7 +38,7 @@ impl Conf { let log_file = conf_file .log_file .clone() - .unwrap_or_else(|| Utf8PathBuf::from("host")) + .unwrap_or_else(|| Utf8PathBuf::from("session")) .pipe_ref(|path| normalize_data_path(path, &data_dir)); Ok(Conf { @@ -237,7 +237,7 @@ pub(crate) mod dto { } pub fn get_data_dir() -> Utf8PathBuf { - if let Ok(config_path_env) = std::env::var("DHOST_DATA_PATH") { + if let Ok(config_path_env) = std::env::var("DSESSION_DATA_PATH") { Utf8PathBuf::from(config_path_env) } else { let mut config_path = Utf8PathBuf::new(); diff --git a/devolutions-host/src/dvc.rs b/devolutions-session/src/dvc.rs similarity index 100% rename from devolutions-host/src/dvc.rs rename to devolutions-session/src/dvc.rs diff --git a/devolutions-host/src/lib.rs b/devolutions-session/src/lib.rs similarity index 100% rename from devolutions-host/src/lib.rs rename to devolutions-session/src/lib.rs diff --git a/devolutions-host/src/log.rs b/devolutions-session/src/log.rs similarity index 73% rename from devolutions-host/src/log.rs rename to devolutions-session/src/log.rs index e66bc55a7..a0b3ad25f 100644 --- a/devolutions-host/src/log.rs +++ b/devolutions-session/src/log.rs @@ -1,18 +1,18 @@ use crate::config::ConfHandle; use devolutions_log::{LoggerGuard, StaticLogConfig}; -pub(crate) struct HostLog; +pub(crate) struct SessionLog; -impl StaticLogConfig for HostLog { +impl StaticLogConfig for SessionLog { const MAX_BYTES_PER_LOG_FILE: u64 = 3_000_000; // 3 MB; const MAX_LOG_FILES: usize = 10; - const LOG_FILE_PREFIX: &'static str = "host"; + const LOG_FILE_PREFIX: &'static str = "session"; } pub fn init_log(config: ConfHandle) -> LoggerGuard { let conf = config.get_conf(); - devolutions_log::init::( + devolutions_log::init::( &conf.log_file, conf.verbosity_profile.to_log_filter(), conf.debug.log_directives.as_deref(), diff --git a/devolutions-host/src/main.rs b/devolutions-session/src/main.rs similarity index 85% rename from devolutions-host/src/main.rs rename to devolutions-session/src/main.rs index d506fc07d..f956181bf 100644 --- a/devolutions-host/src/main.rs +++ b/devolutions-session/src/main.rs @@ -5,10 +5,10 @@ #[macro_use] extern crate tracing; -use devolutions_host::{get_data_dir, init_log, ConfHandle}; +use devolutions_session::{get_data_dir, init_log, ConfHandle}; #[cfg(windows)] -use devolutions_host::loop_dvc; +use devolutions_session::loop_dvc; use anyhow::Context; @@ -23,7 +23,7 @@ fn main() -> anyhow::Result<()> { let _logger_guard = init_log(config.clone()); - info!("Starting Devolutions Host"); + info!("Starting Devolutions Session"); // TMP: Copy-paste from MSRDPEX project for testing purposes. #[cfg(windows)] @@ -40,7 +40,7 @@ fn main() -> anyhow::Result<()> { info!("Waiting for shutdown signal"); shutdown_rx.recv().expect("BUG: Shutdown signal was lost"); - info!("Exiting Devolutions Host"); + info!("Exiting Devolutions Session"); Ok(()) } diff --git a/package.ps1 b/package.ps1 new file mode 100644 index 000000000..e0f2a958c --- /dev/null +++ b/package.ps1 @@ -0,0 +1,40 @@ +$VSINSTALLDIR = $(vswhere.exe -latest -requires Microsoft.VisualStudio.Component.VC.Llvm.Clang -property installationPath) +$Env:LIBCLANG_PATH="$VSINSTALLDIR\VC\Tools\Llvm\x64\bin" +$Env:PATH+=";$Env:ProgramFiles\NASM" + +Enter-VsDevShell + +$PackageVersion = "2024.08.22" +$StagingPath = Join-Path $Env:TEMP "staging" +$SymbolsPath = Join-Path $Env:TEMP "symbols" +New-Item -ItemType Directory $StagingPath -ErrorAction SilentlyContinue | Out-Null +New-Item -ItemType Directory $SymbolsPath -ErrorAction SilentlyContinue | Out-Null + +$TargetPlatform = "windows" +$TargetArch = "x86_64" +$TargetOutputPath = Join-Path $StagingPath $TargetPlatform $TargetArch +New-Item -ItemType Directory $TargetOutputPath -ErrorAction SilentlyContinue | Out-Null +$ExecutableFileName = "DevolutionsAgent_$TargetPlatform_${PackageVersion}_$TargetArch" +$ExecutableFileName = "$($ExecutableFileName).exe" + +$PackageFileName = "DevolutionsAgent-$TargetArch-${PackageVersion}.msi" +$DAgentPackage = Join-Path $TargetOutputPath $PackageFileName +$DAgentExecutable = Join-Path $TargetOutputPath $ExecutableFileName +$DAgentPedmDesktopExecutable = Join-Path $TargetOutputPath "DevolutionsPedmDesktop.exe" +$DAgentPedmShellExtDll = Join-Path $TargetOutputPath "DevolutionsPedmShellExt.dll" +$DAgentPedmShellExtMsix = Join-Path $TargetOutputPath "DevolutionsPedmShellExt.msix" +$DAgentPedmHook = Join-Path $TargetOutputPath "devolutions_pedm_hook.dll" +$DAgentSessionExecutable = Join-Path $TargetOutputPath "DevolutionsSession.exe" + +$DAgentPedmShellExtMsix = "C:\core\DevolutionsPedmShellExt.msix" + +$Env:TARGET_OUTPUT_PATH=$TargetOutputPath +$Env:DAGENT_EXECUTABLE=$DAgentExecutable +$Env:DAGENT_PEDM_DESKTOP_EXECUTABLE = $DAgentPedmDesktopExecutable +$Env:DAGENT_PEDM_HOOK = $DAgentPedmHook +$Env:DAGENT_PEDM_SHELL_EXT_DLL = $DAgentPedmShellExtDll +$Env:DAGENT_PEDM_SHELL_EXT_MSIX = $DAgentPedmShellExtMsix +$Env:DAGENT_SESSION_EXECUTABLE = $DAgentSessionExecutable + +./ci/tlk.ps1 build -Product agent -Platform $TargetPlatform -Architecture $TargetArch -CargoProfile 'release' +./ci/tlk.ps1 package -Product agent -Platform $TargetPlatform -Architecture $TargetArch -CargoProfile 'release' \ No newline at end of file diff --git a/package/AgentWindowsManaged/Actions/AgentActions.cs b/package/AgentWindowsManaged/Actions/AgentActions.cs index 8108a6ae9..126455ff6 100644 --- a/package/AgentWindowsManaged/Actions/AgentActions.cs +++ b/package/AgentWindowsManaged/Actions/AgentActions.cs @@ -252,17 +252,17 @@ internal static class AgentActions Condition = Includes.PEDM_FEATURE.BeingUninstall(), }; - private static readonly ElevatedManagedAction installHost = new( - CustomActions.InstallHost + private static readonly ElevatedManagedAction installSession = new( + CustomActions.InstallSession ) { - Id = new Id("installHost"), - Feature = Includes.PEDM_FEATURE, + Id = new Id("installSession"), + Feature = Includes.SESSION_FEATURE, Sequence = Sequence.InstallExecuteSequence, Return = Return.check, Step = Step.InstallFiles, When = When.After, - Condition = Includes.PEDM_FEATURE.BeingInstall(), + Condition = Includes.SESSION_FEATURE.BeingInstall(), }; private static string UseProperties(IEnumerable properties) @@ -298,7 +298,7 @@ private static string UseProperties(IEnumerable properties) cleanupPedmShellExt, uninstallPedmShellExt, installPedmShellExt, - installHost, + installSession, cleanAgentConfigIfNeeded, cleanAgentConfigIfNeededRollback, diff --git a/package/AgentWindowsManaged/Actions/CustomActions.cs b/package/AgentWindowsManaged/Actions/CustomActions.cs index 6d934ca6e..3a3ea4c79 100644 --- a/package/AgentWindowsManaged/Actions/CustomActions.cs +++ b/package/AgentWindowsManaged/Actions/CustomActions.cs @@ -23,7 +23,7 @@ namespace DevolutionsAgent.Actions public class CustomActions { private static readonly string[] ConfigFiles = new[] { - "agent.json", + "agent.json", }; private const int MAX_PATH = 260; // Defined in windows.h @@ -235,9 +235,9 @@ public static ActionResult InstallPedm(Session session) } [CustomAction] - public static ActionResult InstallHost(Session session) + public static ActionResult InstallSession(Session session) { - return EnableAgentFeature(session, "SessionHost"); + return EnableAgentFeature(session, "Session"); } [CustomAction] @@ -517,14 +517,14 @@ private static ActionResult ExecuteCommand(Session session, string command) { session.Log($"error reading error from temp file: {e}"); } - + using Record record = new(3) { FormatString = "Command execution failure: [1]", }; hTempFile.Close(); - + record.SetString(1, result); session.Message(InstallMessage.Error | (uint)MessageButtons.OK, record); @@ -640,7 +640,7 @@ public static void SetFileSecurity(Session session, string path, string sddl) const uint sdRevision = 1; IntPtr pSd = new IntPtr(); UIntPtr pSzSd = new UIntPtr(); - + try { if (!WinAPI.ConvertStringSecurityDescriptorToSecurityDescriptorW(sddl, sdRevision, out pSd, out pSzSd)) diff --git a/package/AgentWindowsManaged/Program.cs b/package/AgentWindowsManaged/Program.cs index 3f373f7ac..183a5ff69 100644 --- a/package/AgentWindowsManaged/Program.cs +++ b/package/AgentWindowsManaged/Program.cs @@ -119,7 +119,7 @@ private static string ResolveArtifact(string varName, string error) private static string DevolutionsPedmShellExtMsix => ResolveArtifact("DAGENT_PEDM_SHELL_EXT_MSIX", "The PEDM shell extension MSIX was not found"); - private static string DevolutionsHost => ResolveArtifact("DAGENT_DEVOLUTIONS_HOST_EXECUTABLE", "The Devolutions Host executable was not found"); + private static string DevolutionsSession => ResolveArtifact("DAGENT_SESSION_EXECUTABLE", "The Devolutions Session executable was not found"); private static Version DevolutionsAgentVersion { @@ -284,7 +284,7 @@ static void Main() new (Includes.PEDM_FEATURE, DevolutionsPedmHook), new (Includes.PEDM_FEATURE, DevolutionsPedmShellExtDll), new (Includes.PEDM_FEATURE, DevolutionsPedmShellExtMsix), - new (Includes.HOST_FEATURE, DevolutionsHost) + new (Includes.SESSION_FEATURE, DevolutionsSession) }, Dirs = new[] { diff --git a/package/AgentWindowsManaged/Resources/Includes.cs b/package/AgentWindowsManaged/Resources/Includes.cs index 8ef163a10..0721acf12 100644 --- a/package/AgentWindowsManaged/Resources/Includes.cs +++ b/package/AgentWindowsManaged/Resources/Includes.cs @@ -28,7 +28,7 @@ internal static class Includes internal static Feature PEDM_FEATURE = new Feature("Devolutions PEDM", "Installs Devolutions PEDM", false); - internal static Feature HOST_FEATURE = new Feature("Devolutions Host", "Installs Devolutions Host", false); + internal static Feature SESSION_FEATURE = new Feature("Devolutions Session", "Installs Devolutions Session", false); /// /// SDDL string representing desired %programdata%\devolutions\agent ACL diff --git a/tools/bump-version.ps1 b/tools/bump-version.ps1 index 736d2a240..72151c9bc 100755 --- a/tools/bump-version.ps1 +++ b/tools/bump-version.ps1 @@ -15,7 +15,7 @@ $targetFiles = @( './jetsocat/Cargo.toml' './devolutions-gateway/Cargo.toml' './devolutions-agent/Cargo.toml' - './devolutions-host/Cargo.toml' + './devolutions-session/Cargo.toml' './powershell/DevolutionsGateway/DevolutionsGateway.psd1' './Cargo.lock' './fuzz/Cargo.lock' From 45df94726efd8c132c5ee0edaf1cba1d1066aa3b Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Mon, 9 Sep 2024 13:05:24 +0300 Subject: [PATCH 09/10] fix: fixed Process::current_process from win-api-wrappers crate --- crates/win-api-wrappers/src/handle.rs | 12 +++++++ crates/win-api-wrappers/src/process.rs | 6 ++-- devolutions-agent/src/session_manager.rs | 18 +++++------ package.ps1 | 40 ------------------------ 4 files changed, 25 insertions(+), 51 deletions(-) delete mode 100644 package.ps1 diff --git a/crates/win-api-wrappers/src/handle.rs b/crates/win-api-wrappers/src/handle.rs index 1843cf0fb..184eb837b 100644 --- a/crates/win-api-wrappers/src/handle.rs +++ b/crates/win-api-wrappers/src/handle.rs @@ -58,6 +58,18 @@ impl Handle { // SAFETY: Same preconditions as the called function. unsafe { Self::new(handle, true) } } + + /// Wraps a pseudo Windows [`HANDLE`]. + /// + /// # Safety + /// + /// - The caller should ensure that `handle` is a pseudo handle, as its validity is not checked. + pub unsafe fn new_pseudo_handle(handle: HANDLE) -> Self { + Self { + raw: handle, + owned: false, + } + } } impl Handle { diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 239c37bdf..09e171daf 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -176,9 +176,11 @@ impl Process { } pub fn current_process() -> Self { - // SAFETY: `GetCurrentProcess()` has no preconditions and always returns a valid handle. + // SAFETY: `GetCurrentProcess()` has no preconditions and always returns + // a valid pseudo handle. let handle = unsafe { GetCurrentProcess() }; - let handle = Handle::new_borrowed(handle).expect("always valid"); + // SAFETY: The handle returned by `GetCurrentProcess` is a pseudo handle. + let handle = unsafe { Handle::new_pseudo_handle(handle) }; Self { handle } } diff --git a/devolutions-agent/src/session_manager.rs b/devolutions-agent/src/session_manager.rs index 801dc85da..7ff9982d9 100644 --- a/devolutions-agent/src/session_manager.rs +++ b/devolutions-agent/src/session_manager.rs @@ -235,15 +235,15 @@ fn start_session_process_if_not_running(ctx: &mut SessionManagerCtx, session: &S return Ok(()); } - info!(%session, "Starting session process in session"); + info!(%session, "Starting session process"); match start_session_process(session) { Ok(()) => { - info!(%session, "Session process started in session"); + info!(%session, "Session process started"); gw_session.set_session_ready(true); } Err(error) => { - error!(%error, %session, "Failed to start session process for session"); + error!(%error, %session, "Failed to start session process"); } } } @@ -259,13 +259,13 @@ fn start_session_process_if_not_running(ctx: &mut SessionManagerCtx, session: &S fn terminate_session_process(session: &Session) { match terminate_process_by_name_in_session(SESSION_BINARY, session.id) { Ok(false) => { - trace!(%session, "Session process is not running in the session"); + trace!(%session, "Session process is not running"); } Ok(true) => { - info!(%session, "Session process terminated in session"); + info!(%session, "Session process terminated"); } Err(error) => { - error!(%error, %session, "Failed to terminate session process in session"); + error!(%error, %session, "Failed to terminate session process"); } } } @@ -283,7 +283,7 @@ fn start_session_process(session: &Session) -> anyhow::Result<()> { let session_app_path = session_app_path(); let command_line = CommandLine::new(vec!["--session".to_owned(), session.to_string()]); - info!("Starting `{session_app_path}` in session `{session}`"); + info!(%session, "Starting `{session_app_path}`"); let mut startup_info = StartupInfo::default(); @@ -311,11 +311,11 @@ fn start_session_process(session: &Session) -> anyhow::Result<()> { match start_result { Ok(_) => { - info!("{SESSION_BINARY} started in session {session}"); + info!(%session, "{SESSION_BINARY} started"); Ok(()) } Err(error) => { - error!(%error, "Failed to start {SESSION_BINARY} in session {session}"); + error!(%error, %session, "Failed to start {SESSION_BINARY}"); Err(error) } } diff --git a/package.ps1 b/package.ps1 deleted file mode 100644 index e0f2a958c..000000000 --- a/package.ps1 +++ /dev/null @@ -1,40 +0,0 @@ -$VSINSTALLDIR = $(vswhere.exe -latest -requires Microsoft.VisualStudio.Component.VC.Llvm.Clang -property installationPath) -$Env:LIBCLANG_PATH="$VSINSTALLDIR\VC\Tools\Llvm\x64\bin" -$Env:PATH+=";$Env:ProgramFiles\NASM" - -Enter-VsDevShell - -$PackageVersion = "2024.08.22" -$StagingPath = Join-Path $Env:TEMP "staging" -$SymbolsPath = Join-Path $Env:TEMP "symbols" -New-Item -ItemType Directory $StagingPath -ErrorAction SilentlyContinue | Out-Null -New-Item -ItemType Directory $SymbolsPath -ErrorAction SilentlyContinue | Out-Null - -$TargetPlatform = "windows" -$TargetArch = "x86_64" -$TargetOutputPath = Join-Path $StagingPath $TargetPlatform $TargetArch -New-Item -ItemType Directory $TargetOutputPath -ErrorAction SilentlyContinue | Out-Null -$ExecutableFileName = "DevolutionsAgent_$TargetPlatform_${PackageVersion}_$TargetArch" -$ExecutableFileName = "$($ExecutableFileName).exe" - -$PackageFileName = "DevolutionsAgent-$TargetArch-${PackageVersion}.msi" -$DAgentPackage = Join-Path $TargetOutputPath $PackageFileName -$DAgentExecutable = Join-Path $TargetOutputPath $ExecutableFileName -$DAgentPedmDesktopExecutable = Join-Path $TargetOutputPath "DevolutionsPedmDesktop.exe" -$DAgentPedmShellExtDll = Join-Path $TargetOutputPath "DevolutionsPedmShellExt.dll" -$DAgentPedmShellExtMsix = Join-Path $TargetOutputPath "DevolutionsPedmShellExt.msix" -$DAgentPedmHook = Join-Path $TargetOutputPath "devolutions_pedm_hook.dll" -$DAgentSessionExecutable = Join-Path $TargetOutputPath "DevolutionsSession.exe" - -$DAgentPedmShellExtMsix = "C:\core\DevolutionsPedmShellExt.msix" - -$Env:TARGET_OUTPUT_PATH=$TargetOutputPath -$Env:DAGENT_EXECUTABLE=$DAgentExecutable -$Env:DAGENT_PEDM_DESKTOP_EXECUTABLE = $DAgentPedmDesktopExecutable -$Env:DAGENT_PEDM_HOOK = $DAgentPedmHook -$Env:DAGENT_PEDM_SHELL_EXT_DLL = $DAgentPedmShellExtDll -$Env:DAGENT_PEDM_SHELL_EXT_MSIX = $DAgentPedmShellExtMsix -$Env:DAGENT_SESSION_EXECUTABLE = $DAgentSessionExecutable - -./ci/tlk.ps1 build -Product agent -Platform $TargetPlatform -Architecture $TargetArch -CargoProfile 'release' -./ci/tlk.ps1 package -Product agent -Platform $TargetPlatform -Architecture $TargetArch -CargoProfile 'release' \ No newline at end of file From 2d243fb14343b30387c4ad550583e27a2913c51e Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Mon, 9 Sep 2024 13:07:17 +0300 Subject: [PATCH 10/10] refactor: minor renaming --- ci/tlk.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/tlk.ps1 b/ci/tlk.ps1 index 7d3b507d5..a05412e7c 100644 --- a/ci/tlk.ps1 +++ b/ci/tlk.ps1 @@ -504,9 +504,9 @@ class TlkRecipe } if (Test-Path Env:DAGENT_SESSION_EXECUTABLE) { - $hostExe = Get-ChildItem -Recurse -Include 'devolutions-session.exe' | Select-Object -First 1 + $sessionExe = Get-ChildItem -Recurse -Include 'devolutions-session.exe' | Select-Object -First 1 - Copy-Item -Path $hostExe -Destination $Env:DAGENT_SESSION_EXECUTABLE + Copy-Item -Path $sessionExe -Destination $Env:DAGENT_SESSION_EXECUTABLE } }