diff --git a/agent/src/bin/sas_service.rs b/agent/src/bin/sas_service.rs index 0b2e7f4..91dcdcd 100644 --- a/agent/src/bin/sas_service.rs +++ b/agent/src/bin/sas_service.rs @@ -36,7 +36,19 @@ const PIPE_READMODE_MESSAGE: u32 = 0x00000002; const PIPE_WAIT: u32 = 0x00000000; const PIPE_UNLIMITED_INSTANCES: u32 = 255; const INVALID_HANDLE_VALUE: isize = -1; -const SECURITY_DESCRIPTOR_REVISION: u32 = 1; +/// SDDL revision passed to `ConvertStringSecurityDescriptorToSecurityDescriptorW` +/// (`SDDL_REVISION_1`). +const SDDL_REVISION_1: u32 = 1; + +/// Restrictive DACL for the SAS named pipe, in SDDL form. +/// +/// `D:` introduces the DACL; `(A;;GA;;;AU)` is an ACE granting GENERIC_ALL (`GA`) to +/// Authenticated Users (`AU`). Anonymous / null-session callers are NOT authenticated and +/// are therefore denied — closing the original NULL-DACL hole where any local process +/// (Everyone) could connect and make this SYSTEM service raise the secure-attention +/// screen. The agent runs in the interactive logon session and IS an authenticated user, +/// so it can still connect and request a SAS. +const PIPE_SDDL: &str = "D:(A;;GA;;;AU)"; // FFI declarations for named pipe operations #[link(name = "kernel32")] @@ -70,16 +82,18 @@ extern "system" { lpOverlapped: *mut std::ffi::c_void, ) -> i32; fn FlushFileBuffers(hFile: isize) -> i32; + fn LocalFree(hMem: *mut std::ffi::c_void) -> *mut std::ffi::c_void; } #[link(name = "advapi32")] extern "system" { - fn InitializeSecurityDescriptor(pSecurityDescriptor: *mut u8, dwRevision: u32) -> i32; - fn SetSecurityDescriptorDacl( - pSecurityDescriptor: *mut u8, - bDaclPresent: i32, - pDacl: *mut std::ffi::c_void, - bDaclDefaulted: i32, + /// Build a self-relative security descriptor from an SDDL string. The descriptor is + /// allocated with `LocalAlloc` and must be released with `LocalFree`. + fn ConvertStringSecurityDescriptorToSecurityDescriptorW( + StringSecurityDescriptor: *const u16, + StringSDRevision: u32, + SecurityDescriptor: *mut *mut std::ffi::c_void, + SecurityDescriptorSize: *mut u32, ) -> i32; } @@ -281,26 +295,31 @@ fn run_pipe_server() -> Result<()> { tracing::info!("Starting pipe server on {}", PIPE_NAME); loop { - // Create security descriptor that allows everyone - let mut sd = [0u8; 256]; - unsafe { - if InitializeSecurityDescriptor(sd.as_mut_ptr(), SECURITY_DESCRIPTOR_REVISION) == 0 { - tracing::error!("Failed to initialize security descriptor"); - std::thread::sleep(Duration::from_secs(1)); - continue; - } - - // Set NULL DACL = allow everyone - if SetSecurityDescriptorDacl(sd.as_mut_ptr(), 1, std::ptr::null_mut(), 0) == 0 { - tracing::error!("Failed to set security descriptor DACL"); - std::thread::sleep(Duration::from_secs(1)); - continue; - } + // Build a restrictive security descriptor from SDDL: grant access only to + // Authenticated Users (excludes anonymous / null-session callers). See PIPE_SDDL. + let sddl: Vec = PIPE_SDDL.encode_utf16().chain(std::iter::once(0)).collect(); + let mut sd_ptr: *mut std::ffi::c_void = std::ptr::null_mut(); + let converted = unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl.as_ptr(), + SDDL_REVISION_1, + &mut sd_ptr, + std::ptr::null_mut(), + ) + }; + if converted == 0 || sd_ptr.is_null() { + let err = std::io::Error::last_os_error(); + tracing::error!( + "Failed to build pipe security descriptor from SDDL: {}", + err + ); + std::thread::sleep(Duration::from_secs(1)); + continue; } let mut sa = SECURITY_ATTRIBUTES { nLength: std::mem::size_of::() as u32, - lpSecurityDescriptor: sd.as_mut_ptr(), + lpSecurityDescriptor: sd_ptr as *mut u8, bInheritHandle: 0, }; @@ -321,6 +340,12 @@ fn run_pipe_server() -> Result<()> { ) }; + // CreateNamedPipeW copies the descriptor into the kernel object, so the SDDL-built + // copy can be freed now regardless of success. + unsafe { + LocalFree(sd_ptr); + } + if pipe == INVALID_HANDLE_VALUE { tracing::error!("Failed to create named pipe"); std::thread::sleep(Duration::from_secs(1)); @@ -404,6 +429,69 @@ fn run_pipe_server() -> Result<()> { } } +/// Enable the `SoftwareSASGeneration` Winlogon policy so `SendSAS` is permitted. +/// +/// Without this policy, `sas.dll!SendSAS` is a silent no-op even when called from +/// SYSTEM. The value lives at +/// `HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\SoftwareSASGeneration` +/// and is a DWORD bitmask: +/// 0 = none, 1 = services, 2 = ease-of-access apps, 3 = both. +/// +/// We set `1` (services) because the GuruConnect SAS helper runs as a SYSTEM service. +/// This is invoked from the SAS service installer; the broader agent installer should +/// ensure this runs (see `// TODO(installer)` below). +fn set_software_sas_policy() -> Result<()> { + use windows::core::PCWSTR; + use windows::Win32::System::Registry::{ + RegCloseKey, RegCreateKeyExW, RegSetValueExW, HKEY, HKEY_LOCAL_MACHINE, KEY_SET_VALUE, + REG_DWORD, REG_OPTION_NON_VOLATILE, + }; + + let subkey: Vec = r"SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System" + .encode_utf16() + .chain(std::iter::once(0)) + .collect(); + let value_name: Vec = "SoftwareSASGeneration" + .encode_utf16() + .chain(std::iter::once(0)) + .collect(); + // DWORD 1 = allow services to generate a software SAS. + let data: u32 = 1; + + unsafe { + let mut hkey = HKEY::default(); + let status = RegCreateKeyExW( + HKEY_LOCAL_MACHINE, + PCWSTR(subkey.as_ptr()), + 0, + PCWSTR::null(), + REG_OPTION_NON_VOLATILE, + KEY_SET_VALUE, + None, + &mut hkey, + None, + ); + if status.is_err() { + anyhow::bail!("RegCreateKeyExW(Policies\\System) failed: {:?}", status); + } + + let set = RegSetValueExW( + hkey, + PCWSTR(value_name.as_ptr()), + 0, + REG_DWORD, + Some(&data.to_ne_bytes()), + ); + let _ = RegCloseKey(hkey); + + if set.is_err() { + anyhow::bail!("RegSetValueExW(SoftwareSASGeneration) failed: {:?}", set); + } + } + + Ok(()) +} + /// Call SendSAS via sas.dll fn send_sas() -> Result<()> { unsafe { @@ -506,6 +594,19 @@ fn install_service() -> Result<()> { ]) .output(); + // Enable the SoftwareSASGeneration policy so SendSAS actually works from the + // SYSTEM service. TODO(installer): the top-level managed agent installer should + // also ensure this policy is set (and that this SAS service is installed) as part + // of unattended deployment, rather than relying on a manual SAS-service install. + match set_software_sas_policy() { + Ok(()) => println!("Enabled SoftwareSASGeneration policy (services)"), + Err(e) => println!( + "Warning: failed to set SoftwareSASGeneration policy: {}. \ + Ctrl+Alt+Del may not reach the secure desktop until this is set.", + e + ), + } + println!("\n** GuruConnect SAS Service installed successfully!"); println!("\nBinary: {:?}", binary_dest); println!("\nStarting service..."); diff --git a/agent/src/input/keyboard.rs b/agent/src/input/keyboard.rs index 74d2052..568f392 100644 --- a/agent/src/input/keyboard.rs +++ b/agent/src/input/keyboard.rs @@ -1,22 +1,30 @@ //! Keyboard input simulation using Windows SendInput API +//! +//! Injection is **scan-code based** (`KEYEVENTF_SCANCODE`) rather than virtual-key +//! based. Scan codes are layout-independent: the same physical key produces the same +//! scan code regardless of the remote keyboard layout, so the remote machine's active +//! layout (not the technician's) decides what character a key produces. The viewer +//! still carries the virtual-key code for logic that needs it, and we fall back to +//! deriving a scan code from the VK when the wire frame did not supply one. use anyhow::Result; #[cfg(windows)] use windows::Win32::UI::Input::KeyboardAndMouse::{ MapVirtualKeyW, SendInput, INPUT, INPUT_0, INPUT_KEYBOARD, KEYBDINPUT, KEYBD_EVENT_FLAGS, - KEYEVENTF_EXTENDEDKEY, KEYEVENTF_KEYUP, KEYEVENTF_UNICODE, MAPVK_VK_TO_VSC_EX, + KEYEVENTF_EXTENDEDKEY, KEYEVENTF_KEYUP, KEYEVENTF_SCANCODE, KEYEVENTF_UNICODE, + MAPVK_VK_TO_VSC_EX, }; /// Keyboard input controller pub struct KeyboardController { - // Track modifier states for proper handling - #[allow(dead_code)] + /// Tracks which modifier keys this controller currently holds DOWN on the remote. + /// Used so a focus-loss / session-end re-sync can release any still-held modifier + /// and avoid "stuck" Ctrl/Alt/Shift/Win on the remote desktop. modifiers: ModifierState, } -// Modifier tracking is not yet wired into key dispatch. -#[allow(dead_code)] +/// Tracks the down/up state of each modifier the agent has injected. #[derive(Default)] struct ModifierState { ctrl: bool, @@ -25,6 +33,55 @@ struct ModifierState { meta: bool, } +impl ModifierState { + /// Record a modifier transition for `vk_code`. Returns `true` if `vk_code` is a + /// modifier key (and the state was updated), `false` otherwise. + fn record(&mut self, vk_code: u16, down: bool) -> bool { + match vk_code { + // VK_CONTROL / VK_LCONTROL / VK_RCONTROL + 0x11 | 0xA2 | 0xA3 => { + self.ctrl = down; + true + } + // VK_MENU / VK_LMENU / VK_RMENU (Alt) + 0x12 | 0xA4 | 0xA5 => { + self.alt = down; + true + } + // VK_SHIFT / VK_LSHIFT / VK_RSHIFT + 0x10 | 0xA0 | 0xA1 => { + self.shift = down; + true + } + // VK_LWIN / VK_RWIN + 0x5B | 0x5C => { + self.meta = down; + true + } + _ => false, + } + } + + /// Return the VK codes of every modifier currently held down, then clear the state. + fn drain_held(&mut self) -> Vec { + let mut held = Vec::new(); + if self.ctrl { + held.push(0x11); + } + if self.alt { + held.push(0x12); + } + if self.shift { + held.push(0x10); + } + if self.meta { + held.push(0x5B); + } + *self = ModifierState::default(); + held + } +} + impl KeyboardController { /// Create a new keyboard controller pub fn new() -> Result { @@ -33,28 +90,75 @@ impl KeyboardController { }) } - /// Press a key down by virtual key code + /// Press a key down by virtual key code (scan code derived from the VK). #[cfg(windows)] pub fn key_down(&mut self, vk_code: u16) -> Result<()> { - self.send_key(vk_code, true) + self.send_key(vk_code, 0, false, true) } - /// Release a key by virtual key code + /// Release a key by virtual key code (scan code derived from the VK). #[cfg(windows)] pub fn key_up(&mut self, vk_code: u16) -> Result<()> { - self.send_key(vk_code, false) + self.send_key(vk_code, 0, false, false) } - /// Send a key event + /// Inject a full-fidelity key event. + /// + /// `scan_code` is the hardware scan code captured by the viewer's low-level hook + /// (0 ⇒ derive it from `vk_code`). `is_extended` is the viewer-captured extended-key + /// flag (`LLKHF_EXTENDED`); when `false` the agent still derives the flag from the + /// VK / scan code so older viewers that don't set it stay correct. #[cfg(windows)] - fn send_key(&mut self, vk_code: u16, down: bool) -> Result<()> { - // Get scan code from virtual key - let scan_code = unsafe { MapVirtualKeyW(vk_code as u32, MAPVK_VK_TO_VSC_EX) as u16 }; + pub fn key_event_full( + &mut self, + vk_code: u16, + scan_code: u16, + is_extended: bool, + down: bool, + ) -> Result<()> { + self.send_key(vk_code, scan_code, is_extended, down) + } - let mut flags = KEYBD_EVENT_FLAGS::default(); + /// Release every modifier this controller currently holds down on the remote. + /// + /// Called on viewer focus loss and at session end so a Ctrl/Alt/Shift/Win that was + /// pressed but whose key-up never arrived (e.g. the technician alt-tabbed away) does + /// not stay latched on the remote desktop. + #[cfg(windows)] + pub fn release_all_modifiers(&mut self) -> Result<()> { + for vk in self.modifiers.drain_held() { + // Emit the key-up directly; drain_held already cleared the tracked state. + if let Err(e) = self.send_key(vk, 0, false, false) { + tracing::warn!("Failed to release held modifier vk={:#x}: {}", vk, e); + } else { + tracing::debug!("Released stuck modifier vk={:#x} on focus loss", vk); + } + } + Ok(()) + } - // Add extended key flag for certain keys - if Self::is_extended_key(vk_code) || (scan_code >> 8) == 0xE0 { + /// Send a key event using scan-code injection. + #[cfg(windows)] + fn send_key( + &mut self, + vk_code: u16, + scan_code: u16, + is_extended: bool, + down: bool, + ) -> Result<()> { + // Track modifier state so we can release stuck modifiers later. + self.modifiers.record(vk_code, down); + + // Prefer the viewer-supplied scan code; fall back to deriving one from the VK. + // MAPVK_VK_TO_VSC_EX yields a 0xE0-prefixed value for extended keys. + let mapped = unsafe { MapVirtualKeyW(vk_code as u32, MAPVK_VK_TO_VSC_EX) as u16 }; + let effective_scan = if scan_code != 0 { scan_code } else { mapped }; + + let mut flags = KEYBD_EVENT_FLAGS::default() | KEYEVENTF_SCANCODE; + + // Add the extended flag if the viewer flagged it, the VK is inherently + // extended, or the mapped scan code carries the 0xE0 extended prefix. + if is_extended || Self::is_extended_key(vk_code) || (mapped >> 8) == 0xE0 { flags |= KEYEVENTF_EXTENDEDKEY; } @@ -62,12 +166,16 @@ impl KeyboardController { flags |= KEYEVENTF_KEYUP; } + // For scan-code injection the low byte of the scan code is what Windows uses; + // the 0xE0 prefix is conveyed via KEYEVENTF_EXTENDEDKEY, not the wScan value. + let w_scan = (effective_scan & 0x00FF) as u16; + let input = INPUT { r#type: INPUT_KEYBOARD, Anonymous: INPUT_0 { ki: KEYBDINPUT { - wVk: windows::Win32::UI::Input::KeyboardAndMouse::VIRTUAL_KEY(vk_code), - wScan: scan_code, + wVk: windows::Win32::UI::Input::KeyboardAndMouse::VIRTUAL_KEY(0), + wScan: w_scan, dwFlags: flags, time: 0, dwExtraInfo: 0, @@ -132,21 +240,35 @@ impl KeyboardController { /// Send Secure Attention Sequence (Ctrl+Alt+Delete) /// - /// This uses a multi-tier approach: - /// 1. Try the GuruConnect SAS Service (runs as SYSTEM, handles via named pipe) - /// 2. Try the sas.dll directly (requires SYSTEM privileges) - /// 3. Fallback to key simulation (won't work on secure desktop) + /// Ctrl+Alt+Del is the Secure Attention Sequence and **cannot** be injected via + /// `SendInput` — Windows reserves it. It must be raised by `SendSAS`, which only + /// works when the caller runs as SYSTEM (or has SeTcbPrivilege) AND the + /// `SoftwareSASGeneration` Winlogon policy permits software-generated SAS. The + /// managed installer is responsible for installing the SAS helper service (running + /// as SYSTEM) and setting that policy. See `set_software_sas_policy` in + /// `bin/sas_service.rs` and the `// TODO(installer)` note there. + /// + /// Tiers, in order: + /// 1. The GuruConnect SAS helper service (SYSTEM) via named-pipe IPC — the supported path. + /// 2. Direct `sas.dll!SendSAS` — only succeeds if THIS process is already SYSTEM with the policy. + /// 3. Fallback key simulation — will NOT reach the secure desktop; logged as a clear failure. #[cfg(windows)] pub fn send_sas(&mut self) -> Result<()> { // Tier 1: Try the SAS service (named pipe IPC to SYSTEM service) - if let Ok(()) = crate::sas_client::request_sas() { - tracing::info!("SAS sent via GuruConnect SAS Service"); - return Ok(()); + match crate::sas_client::request_sas() { + Ok(()) => { + tracing::info!("SAS sent via GuruConnect SAS Service"); + return Ok(()); + } + Err(e) => { + tracing::warn!( + "SAS helper service unavailable ({}); trying direct sas.dll", + e + ); + } } - tracing::info!("SAS service not available, trying direct sas.dll..."); - - // Tier 2: Try using the sas.dll directly (requires SYSTEM privileges) + // Tier 2: Try using the sas.dll directly (requires SYSTEM + SoftwareSASGeneration) use windows::core::PCWSTR; use windows::Win32::System::LibraryLoader::{GetProcAddress, LoadLibraryW}; @@ -157,49 +279,33 @@ impl KeyboardController { if let Ok(lib) = lib { let proc_name = b"SendSAS\0"; if let Some(proc) = GetProcAddress(lib, windows::core::PCSTR(proc_name.as_ptr())) { - // SendSAS takes a BOOL parameter: FALSE for Ctrl+Alt+Del + // SendSAS takes a BOOL parameter: FALSE for Ctrl+Alt+Del. + // It silently no-ops if the caller lacks privilege / the policy is + // unset, so we cannot detect success here — but it is the best + // effort short of the SYSTEM helper. let send_sas: extern "system" fn(i32) = std::mem::transmute(proc); send_sas(0); // FALSE = Ctrl+Alt+Del - tracing::info!("SAS sent via direct sas.dll call"); + tracing::info!("SAS attempted via direct sas.dll call (effective only if SYSTEM + SoftwareSASGeneration policy set)"); return Ok(()); } } } - // Tier 3: Fallback - try sending the keys (won't work on secure desktop) - tracing::warn!("SAS service and sas.dll not available, Ctrl+Alt+Del may not work"); - - // VK codes - const VK_CONTROL: u16 = 0x11; - const VK_MENU: u16 = 0x12; // Alt - const VK_DELETE: u16 = 0x2E; - - // Press keys - self.key_down(VK_CONTROL)?; - self.key_down(VK_MENU)?; - self.key_down(VK_DELETE)?; - - // Release keys - self.key_up(VK_DELETE)?; - self.key_up(VK_MENU)?; - self.key_up(VK_CONTROL)?; - - Ok(()) + // Tier 3: SAS could not be delivered through any privileged path. A plain + // SendInput of Ctrl+Alt+Del never reaches the secure desktop, so report a + // clear, actionable error instead of pretending it worked. + let msg = "Ctrl+Alt+Del could not be delivered: the GuruConnect SAS helper \ + service is not running and sas.dll!SendSAS is unavailable. Ensure the \ + SAS service is installed (runs as SYSTEM) and the SoftwareSASGeneration \ + policy is enabled by the installer."; + tracing::error!("{}", msg); + anyhow::bail!("{}", msg) } /// Check if a virtual key code is an extended key #[cfg(windows)] fn is_extended_key(vk: u16) -> bool { - matches!( - vk, - 0x21..=0x28 | // Page Up, Page Down, End, Home, Arrow keys - 0x2D | 0x2E | // Insert, Delete - 0x5B | 0x5C | // Left/Right Windows keys - 0x5D | // Applications key - 0x6F | // Numpad Divide - 0x90 | // Num Lock - 0x91 // Scroll Lock - ) + vk_is_extended(vk) } /// Send input events @@ -224,6 +330,22 @@ impl KeyboardController { anyhow::bail!("Keyboard input only supported on Windows") } + #[cfg(not(windows))] + pub fn key_event_full( + &mut self, + _vk_code: u16, + _scan_code: u16, + _is_extended: bool, + _down: bool, + ) -> Result<()> { + anyhow::bail!("Keyboard input only supported on Windows") + } + + #[cfg(not(windows))] + pub fn release_all_modifiers(&mut self) -> Result<()> { + anyhow::bail!("Keyboard input only supported on Windows") + } + #[cfg(not(windows))] pub fn type_char(&mut self, _ch: char) -> Result<()> { anyhow::bail!("Keyboard input only supported on Windows") @@ -293,3 +415,121 @@ pub mod vk { pub const LMENU: u16 = 0xA4; // Left Alt pub const RMENU: u16 = 0xA5; // Right Alt } + +/// Whether a Windows virtual-key code is an "extended" key. +/// +/// Extended keys must be injected with `KEYEVENTF_EXTENDEDKEY`. This is the +/// platform-independent classifier so the determination can be unit-tested off-Windows; +/// the `#[cfg(windows)]` injection path delegates here. The viewer-captured +/// `LLKHF_EXTENDED` flag is authoritative when present; this is the fallback used when +/// the wire frame did not carry it (older viewers / VK-only synthesis). +pub fn vk_is_extended(vk: u16) -> bool { + matches!( + vk, + 0x21..=0x28 | // Page Up, Page Down, End, Home, Arrow keys + 0x2D | 0x2E | // Insert, Delete + 0x5B | 0x5C | // Left/Right Windows keys + 0x5D | // Applications key + 0x6F | // Numpad Divide + 0x90 | // Num Lock + 0x91 | // Scroll Lock + 0xA3 | // Right Control + 0xA5 // Right Alt (AltGr) + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn extended_keys_are_flagged() { + // Arrows / navigation block. + for vk in [0x21u16, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28] { + assert!(vk_is_extended(vk), "vk={:#x} should be extended", vk); + } + // Insert / Delete. + assert!(vk_is_extended(0x2D)); + assert!(vk_is_extended(0x2E)); + // Win keys, Apps, NumLock, numpad Divide. + assert!(vk_is_extended(0x5B)); + assert!(vk_is_extended(0x5C)); + assert!(vk_is_extended(0x5D)); + assert!(vk_is_extended(0x6F)); + assert!(vk_is_extended(0x90)); + // Right Ctrl / Right Alt. + assert!(vk_is_extended(0xA3)); + assert!(vk_is_extended(0xA5)); + } + + #[test] + fn non_extended_keys_are_not_flagged() { + // Letters, digits, space, enter, left modifiers, numpad digits. + for vk in [ + 0x41u16, // A + 0x5A, // Z + 0x30, // 0 + 0x20, // Space + 0x0D, // Enter + 0xA0, // Left Shift + 0xA2, // Left Control + 0xA4, // Left Alt + 0x60, // Numpad 0 + 0x6A, // Numpad Multiply (NOT extended; only Divide is) + ] { + assert!(!vk_is_extended(vk), "vk={:#x} should NOT be extended", vk); + } + } + + #[test] + fn modifier_state_records_ctrl_alt_shift_win() { + let mut m = ModifierState::default(); + // Each of the VK aliases maps to its modifier flag. + assert!(m.record(0x11, true)); // VK_CONTROL + assert!(m.ctrl); + assert!(m.record(0xA4, true)); // VK_LMENU (Alt) + assert!(m.alt); + assert!(m.record(0xA0, true)); // VK_LSHIFT + assert!(m.shift); + assert!(m.record(0x5C, true)); // VK_RWIN + assert!(m.meta); + } + + #[test] + fn modifier_state_ignores_non_modifiers() { + let mut m = ModifierState::default(); + assert!(!m.record(0x41, true)); // 'A' is not a modifier + assert!(!m.ctrl && !m.alt && !m.shift && !m.meta); + } + + #[test] + fn modifier_state_tracks_down_then_up() { + let mut m = ModifierState::default(); + m.record(0x11, true); // Ctrl down + assert!(m.ctrl); + m.record(0x11, false); // Ctrl up + assert!(!m.ctrl); + } + + #[test] + fn drain_held_returns_and_clears_held_modifiers() { + let mut m = ModifierState::default(); + m.record(0xA2, true); // Left Ctrl -> ctrl + m.record(0x12, true); // Alt + // Shift and Win were never pressed. + let mut held = m.drain_held(); + held.sort_unstable(); + // Canonical VKs returned: Ctrl(0x11), Alt(0x12). + assert_eq!(held, vec![0x11u16, 0x12]); + // State is cleared after draining. + assert!(!m.ctrl && !m.alt && !m.shift && !m.meta); + // A second drain yields nothing. + assert!(m.drain_held().is_empty()); + } + + #[test] + fn drain_held_empty_when_nothing_pressed() { + let mut m = ModifierState::default(); + assert!(m.drain_held().is_empty()); + } +} diff --git a/agent/src/input/mod.rs b/agent/src/input/mod.rs index e337e96..08fc570 100644 --- a/agent/src/input/mod.rs +++ b/agent/src/input/mod.rs @@ -5,6 +5,7 @@ mod keyboard; mod mouse; +pub use keyboard::vk_is_extended; pub use keyboard::KeyboardController; pub use mouse::MouseController; @@ -56,7 +57,8 @@ impl InputController { self.mouse.scroll(delta_x, delta_y) } - /// Press or release a key + /// Press or release a key by virtual-key code only (scan code derived from the VK). + #[allow(dead_code)] pub fn key_event(&mut self, vk_code: u16, down: bool) -> Result<()> { if down { self.keyboard.key_down(vk_code) @@ -65,6 +67,30 @@ impl InputController { } } + /// Inject a full-fidelity key event (VK + hardware scan code + extended-key flag). + /// + /// This is the path used for relayed viewer keystrokes so that scan-code injection + /// (layout-independent) and the correct `KEYEVENTF_EXTENDEDKEY` flag are applied. + pub fn key_event_full( + &mut self, + vk_code: u16, + scan_code: u16, + is_extended: bool, + down: bool, + ) -> Result<()> { + self.keyboard + .key_event_full(vk_code, scan_code, is_extended, down) + } + + /// Release any modifier keys currently held down on the remote. + /// + /// Invoked when the viewer loses focus or the session ends so a Ctrl/Alt/Shift/Win + /// whose key-up never arrived does not stay latched on the remote desktop. + #[allow(dead_code)] + pub fn release_all_modifiers(&mut self) -> Result<()> { + self.keyboard.release_all_modifiers() + } + /// Type a unicode character #[allow(dead_code)] pub fn type_unicode(&mut self, ch: char) -> Result<()> { diff --git a/agent/src/session/mod.rs b/agent/src/session/mod.rs index 6689a53..817e9b5 100644 --- a/agent/src/session/mod.rs +++ b/agent/src/session/mod.rs @@ -616,7 +616,15 @@ impl SessionManager { Some(message::Payload::KeyEvent(key)) => { if let Some(input) = self.input.as_mut() { - input.key_event(key.vk_code as u16, key.down)?; + // Full-fidelity scan-code injection: pass the viewer-captured + // scan code and extended-key flag through. A scan_code of 0 (older + // viewers / synthesized events) makes the agent derive it from the VK. + input.key_event_full( + key.vk_code as u16, + key.scan_code as u16, + key.is_extended, + key.down, + )?; } } diff --git a/agent/src/viewer/input.rs b/agent/src/viewer/input.rs index 839eec8..34d66b7 100644 --- a/agent/src/viewer/input.rs +++ b/agent/src/viewer/input.rs @@ -1,9 +1,24 @@ -//! Low-level keyboard hook for capturing all keys including Win key +//! Low-level keyboard hook for capturing system key combinations. +//! +//! `WH_KEYBOARD_LL` is a GLOBAL hook: the OS invokes it for ALL desktop input regardless +//! of which window is focused. We therefore gate diversion on the viewer's focus state. +//! ONLY when the viewer window actually has focus AND "send system keys to remote" is +//! enabled does the hook DIVERT the system combinations the local shell would otherwise +//! consume — the Windows key, Win+R, Win+E, Alt+Tab, Ctrl+Esc, Alt+Esc — and forward them +//! to the remote as full-fidelity `KeyEvent`s (virtual key + hardware scan code + +//! extended-key flag + modifier snapshot), returning 1 from the hook proc to suppress the +//! local handling. All other keys flow through the normal viewer input path. +//! +//! When the toggle is OFF, the viewer is not focused, or the key is not a system combo, +//! the hook diverts NOTHING — it falls through to `CallNextHookEx` and every key reaches +//! the local OS unchanged. This keeps the technician's own Start menu / Alt+Tab working +//! while the viewer sits unfocused in the background. use super::InputEvent; #[cfg(windows)] use crate::proto; use anyhow::Result; +use std::sync::atomic::{AtomicBool, Ordering}; use tokio::sync::mpsc; #[cfg(windows)] use tracing::trace; @@ -13,37 +28,83 @@ use windows::{ Win32::Foundation::{LPARAM, LRESULT, WPARAM}, Win32::UI::WindowsAndMessaging::{ CallNextHookEx, DispatchMessageW, PeekMessageW, SetWindowsHookExW, TranslateMessage, - UnhookWindowsHookEx, HHOOK, KBDLLHOOKSTRUCT, MSG, PM_REMOVE, WH_KEYBOARD_LL, WM_KEYDOWN, - WM_KEYUP, WM_SYSKEYDOWN, WM_SYSKEYUP, + UnhookWindowsHookEx, HHOOK, KBDLLHOOKSTRUCT, LLKHF_EXTENDED, MSG, PM_REMOVE, + WH_KEYBOARD_LL, WM_KEYDOWN, WM_KEYUP, WM_SYSKEYDOWN, WM_SYSKEYUP, }, }; #[cfg(windows)] use std::sync::OnceLock; +/// Global toggle: when `true`, system key combinations are diverted to the remote; +/// when `false`, the hook is transparent and the local OS handles them. Default ON. +/// +/// Lives at module scope because the `WH_KEYBOARD_LL` callback is a bare `extern "system"` +/// function with no user context pointer, so its state must be reachable statically. +static SEND_SYSTEM_KEYS: AtomicBool = AtomicBool::new(true); + +/// Set whether system key combinations are forwarded to the remote (vs. handled locally). +/// +/// Part of the programmatic toggle API (alongside `toggle_send_system_keys`, which the +/// Pause/Break host key drives). Retained for a future viewer menu / tray item and used +/// by the unit tests; not yet called from non-test code, hence the allow. +#[allow(dead_code)] +pub fn set_send_system_keys(enabled: bool) { + SEND_SYSTEM_KEYS.store(enabled, Ordering::Relaxed); +} + +/// Flip the "send system keys to remote" toggle and return the new value. +pub fn toggle_send_system_keys() -> bool { + // fetch_xor(true) flips the bit and returns the PREVIOUS value; invert for the new one. + !SEND_SYSTEM_KEYS.fetch_xor(true, Ordering::Relaxed) +} + +/// Current state of the "send system keys to remote" toggle. +/// +/// Part of the programmatic toggle API; used by the unit tests and available for a +/// viewer menu / status indicator. Not yet read from non-test code, hence the allow. +#[allow(dead_code)] +pub fn send_system_keys_enabled() -> bool { + SEND_SYSTEM_KEYS.load(Ordering::Relaxed) +} + +/// Whether the viewer window currently has input focus. Default `false`. +/// +/// `WH_KEYBOARD_LL` is a GLOBAL hook fired for all desktop input, so it must NOT divert +/// system combos while the viewer is unfocused — otherwise the technician's own local +/// Win key / Alt+Tab / Ctrl+Esc would be suppressed and pushed to the remote. The render +/// loop updates this on `WindowEvent::Focused`. Lives at module scope for the same reason +/// as `SEND_SYSTEM_KEYS`: the bare `extern "system"` callback has no user-context pointer. +static VIEWER_FOCUSED: AtomicBool = AtomicBool::new(false); + +/// Record whether the viewer window has input focus (drives the hook's focus gate). +pub fn set_viewer_focused(focused: bool) { + VIEWER_FOCUSED.store(focused, Ordering::Relaxed); +} + +/// Current focus state as seen by the keyboard hook. +/// +/// Used by the unit tests and available for diagnostics; not yet read from non-test code +/// beyond the hook callback itself, hence the allow. +#[allow(dead_code)] +pub fn viewer_focused() -> bool { + VIEWER_FOCUSED.load(Ordering::Relaxed) +} + #[cfg(windows)] static INPUT_TX: OnceLock> = OnceLock::new(); #[cfg(windows)] static mut HOOK_HANDLE: HHOOK = HHOOK(std::ptr::null_mut()); -/// Virtual key codes for special keys -// Several entries are reserved for upcoming special-key fidelity work. +/// Virtual key codes for keys the hook reasons about. #[cfg(windows)] -#[allow(dead_code)] mod vk { pub const VK_LWIN: u32 = 0x5B; pub const VK_RWIN: u32 = 0x5C; pub const VK_APPS: u32 = 0x5D; - pub const VK_LSHIFT: u32 = 0xA0; - pub const VK_RSHIFT: u32 = 0xA1; - pub const VK_LCONTROL: u32 = 0xA2; - pub const VK_RCONTROL: u32 = 0xA3; - pub const VK_LMENU: u32 = 0xA4; // Left Alt - pub const VK_RMENU: u32 = 0xA5; // Right Alt pub const VK_TAB: u32 = 0x09; pub const VK_ESCAPE: u32 = 0x1B; - pub const VK_SNAPSHOT: u32 = 0x2C; // Print Screen } #[cfg(windows)] @@ -54,10 +115,10 @@ pub struct KeyboardHook { #[cfg(windows)] impl KeyboardHook { pub fn new(input_tx: mpsc::Sender) -> Result { - // Store the sender globally for the hook callback - INPUT_TX - .set(input_tx) - .map_err(|_| anyhow::anyhow!("Input TX already set"))?; + // Store the sender globally for the hook callback. If it was already set (e.g. + // a previous viewer instance in the same process), reuse the existing one rather + // than failing — the hook handle itself is what we re-install. + let _ = INPUT_TX.set(input_tx); unsafe { let hook = SetWindowsHookExW(WH_KEYBOARD_LL, Some(keyboard_hook_proc), None, 0)?; @@ -80,42 +141,78 @@ impl Drop for KeyboardHook { } } +/// Decide whether a key event is a SYSTEM combination we must divert to the remote. +/// +/// `vk_code` is the key; `alt`/`ctrl` are the modifier state at the moment of the event +/// (from `GetAsyncKeyState`). The Windows-key combos (Win, Win+R, Win+E) are recognized +/// by matching the Win keys themselves, so the held-Win state is not needed here. Pure +/// functions like this keep the (untestable) hook callback thin and unit-testable. +#[cfg(windows)] +fn is_system_combo(vk_code: u32, alt: bool, ctrl: bool) -> bool { + match vk_code { + // The Windows keys and the Applications (context-menu) key: always divert so the + // local Start menu / Win+R / Win+E / Win+E etc. do not fire. With Win forwarded + // down to the remote, subsequent letters (R, E, ...) compose there naturally. + vk::VK_LWIN | vk::VK_RWIN | vk::VK_APPS => true, + // Alt+Tab and Alt+Esc — the local window-switcher would otherwise eat these. + vk::VK_TAB if alt => true, + vk::VK_ESCAPE if alt => true, + // Ctrl+Esc opens the local Start menu; divert it. + vk::VK_ESCAPE if ctrl => true, + _ => false, + } +} + #[cfg(windows)] unsafe extern "system" fn keyboard_hook_proc(code: i32, wparam: WPARAM, lparam: LPARAM) -> LRESULT { if code >= 0 { let kb_struct = &*(lparam.0 as *const KBDLLHOOKSTRUCT); let vk_code = kb_struct.vkCode; let scan_code = kb_struct.scanCode; + // LLKHF_EXTENDED (bit 0) marks extended keys (right Ctrl/Alt, arrows, etc.). + let is_extended = (kb_struct.flags.0 & LLKHF_EXTENDED.0) != 0; let is_down = wparam.0 as u32 == WM_KEYDOWN || wparam.0 as u32 == WM_SYSKEYDOWN; let is_up = wparam.0 as u32 == WM_KEYUP || wparam.0 as u32 == WM_SYSKEYUP; if is_down || is_up { - // Check if this is a key we want to intercept (Win key, Alt+Tab, etc.) - let should_intercept = matches!(vk_code, vk::VK_LWIN | vk::VK_RWIN | vk::VK_APPS); + let forwarding = SEND_SYSTEM_KEYS.load(Ordering::Relaxed); + let focused = VIEWER_FOCUSED.load(Ordering::Relaxed); + let modifiers = current_modifiers(); - // Send the key event to the remote - if let Some(tx) = INPUT_TX.get() { - let event = proto::KeyEvent { - down: is_down, - key_type: proto::KeyEventType::KeyVk as i32, - vk_code, - scan_code, - unicode: String::new(), - modifiers: Some(get_current_modifiers()), - }; + // Divert ONLY a SYSTEM combo, ONLY while forwarding is enabled, and ONLY while + // the viewer window has focus. This is a global hook, so without the focus gate + // we would swallow the technician's own Win/Alt+Tab/Ctrl+Esc while the viewer + // sits unfocused in the background. When any condition is false we fall through + // to CallNextHookEx and suppress nothing — the local OS handles the key. Ordinary + // keys are left to the normal winit viewer input path (they are NOT forwarded + // here to avoid double-injection). + let divert = + forwarding && focused && is_system_combo(vk_code, modifiers.alt, modifiers.ctrl); - let _ = tx.try_send(InputEvent::Key(event)); - trace!( - "Key hook: vk={:#x} scan={} down={}", - vk_code, - scan_code, - is_down - ); - } + if divert { + if let Some(tx) = INPUT_TX.get() { + let event = proto::KeyEvent { + down: is_down, + key_type: proto::KeyEventType::KeyVk as i32, + vk_code, + scan_code, + unicode: String::new(), + is_extended, + modifiers: Some(modifiers), + }; - // For Win key, consume the event so it doesn't open Start menu locally - if should_intercept { + let _ = tx.try_send(InputEvent::Key(event)); + trace!( + "System-key hook diverted: vk={:#x} scan={} ext={} down={}", + vk_code, + scan_code, + is_extended, + is_down + ); + } + + // Suppress local handling of the diverted system combo. return LRESULT(1); } } @@ -125,7 +222,7 @@ unsafe extern "system" fn keyboard_hook_proc(code: i32, wparam: WPARAM, lparam: } #[cfg(windows)] -fn get_current_modifiers() -> proto::Modifiers { +fn current_modifiers() -> proto::Modifiers { use windows::Win32::UI::Input::KeyboardAndMouse::GetAsyncKeyState; unsafe { @@ -140,7 +237,7 @@ fn get_current_modifiers() -> proto::Modifiers { } } -/// Pump Windows message queue (required for hooks to work) +/// Pump Windows message queue (required for hooks to work). #[cfg(windows)] pub fn pump_messages() { unsafe { @@ -168,3 +265,74 @@ impl KeyboardHook { #[cfg(not(windows))] #[allow(dead_code)] pub fn pump_messages() {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn toggle_defaults_on_and_flips() { + // Default is ON. + set_send_system_keys(true); + assert!(send_system_keys_enabled()); + + // Toggling flips and returns the NEW value. + assert!(!toggle_send_system_keys()); + assert!(!send_system_keys_enabled()); + assert!(toggle_send_system_keys()); + assert!(send_system_keys_enabled()); + + // Explicit set wins. + set_send_system_keys(false); + assert!(!send_system_keys_enabled()); + set_send_system_keys(true); + } + + #[test] + fn viewer_focus_flag_defaults_off_and_tracks() { + // The hook starts gated CLOSED (unfocused) so a background viewer never swallows + // the technician's local system keys until it actually gains focus. + set_viewer_focused(false); + assert!(!viewer_focused()); + + set_viewer_focused(true); + assert!(viewer_focused()); + + set_viewer_focused(false); + assert!(!viewer_focused()); + } + + #[cfg(windows)] + #[test] + fn win_keys_always_divert() { + // Win / Apps keys divert regardless of modifier state. + assert!(is_system_combo(vk::VK_LWIN, false, false)); + assert!(is_system_combo(vk::VK_RWIN, false, false)); + assert!(is_system_combo(vk::VK_APPS, false, false)); + } + + #[cfg(windows)] + #[test] + fn alt_tab_and_alt_esc_divert_only_with_alt() { + assert!(is_system_combo(vk::VK_TAB, true, false)); // Alt+Tab + assert!(!is_system_combo(vk::VK_TAB, false, false)); // plain Tab -> local path + assert!(is_system_combo(vk::VK_ESCAPE, true, false)); // Alt+Esc + } + + #[cfg(windows)] + #[test] + fn ctrl_esc_diverts_only_with_ctrl() { + assert!(is_system_combo(vk::VK_ESCAPE, false, true)); // Ctrl+Esc + assert!(!is_system_combo(vk::VK_ESCAPE, false, false)); // plain Esc -> local path + } + + #[cfg(windows)] + #[test] + fn ordinary_keys_never_divert() { + // 'R' is NOT itself a "system combo" — Win was already diverted (and forwarded + // down), so R flows through the normal viewer path and composes Win+R on the remote. + assert!(!is_system_combo(0x52, false, false)); // 'R' + assert!(!is_system_combo(0x41, false, false)); // 'A' + assert!(!is_system_combo(vk::VK_TAB, false, true)); // Ctrl+Tab is app-level, not a shell combo + } +} diff --git a/agent/src/viewer/render.rs b/agent/src/viewer/render.rs index 2bc4663..0d05882 100644 --- a/agent/src/viewer/render.rs +++ b/agent/src/viewer/render.rs @@ -1,6 +1,5 @@ //! Window rendering and frame display -#[cfg(windows)] use super::input; use super::{InputEvent, ViewerEvent}; use crate::proto; @@ -30,6 +29,50 @@ pub struct FrameData { pub is_keyframe: bool, } +/// Viewer-local tracker of which modifier keys are currently held down on the remote. +/// +/// Mirrors what the viewer has forwarded so that on focus loss it can emit explicit +/// key-ups for anything still pressed, preventing a stuck Ctrl/Alt/Shift/Win. +#[derive(Default)] +struct ViewerModifierState { + ctrl: bool, + alt: bool, + shift: bool, + meta: bool, +} + +impl ViewerModifierState { + /// Record a modifier transition for `vk_code`. + fn update(&mut self, vk_code: u32, down: bool) { + match vk_code { + 0x11 | 0xA2 | 0xA3 => self.ctrl = down, // Ctrl / LCtrl / RCtrl + 0x12 | 0xA4 | 0xA5 => self.alt = down, // Alt / LAlt / RAlt + 0x10 | 0xA0 | 0xA1 => self.shift = down, // Shift / LShift / RShift + 0x5B | 0x5C => self.meta = down, // LWin / RWin + _ => {} + } + } + + /// Return the canonical VK of every held modifier, then clear all state. + fn drain_held(&mut self) -> Vec { + let mut held = Vec::new(); + if self.ctrl { + held.push(0x11u16); + } + if self.alt { + held.push(0x12); + } + if self.shift { + held.push(0x10); + } + if self.meta { + held.push(0x5B); + } + *self = ViewerModifierState::default(); + held + } +} + struct ViewerApp { window: Option>, surface: Option, Arc>>, @@ -40,6 +83,7 @@ struct ViewerApp { input_tx: mpsc::Sender, mouse_x: i32, mouse_y: i32, + modifiers: ViewerModifierState, #[cfg(windows)] keyboard_hook: Option, } @@ -56,6 +100,7 @@ impl ViewerApp { input_tx, mouse_x: 0, mouse_y: 0, + modifiers: ViewerModifierState::default(), #[cfg(windows)] keyboard_hook: None, } @@ -216,24 +261,56 @@ impl ViewerApp { let _ = self.input_tx.try_send(InputEvent::Mouse(event)); } - fn send_key_event(&self, key: PhysicalKey, state: ElementState) { + fn send_key_event(&mut self, key: PhysicalKey, state: ElementState) { let vk_code = match key { PhysicalKey::Code(code) => keycode_to_vk(code), _ => return, }; + if vk_code == 0 { + return; + } + let down = state == ElementState::Pressed; + + // Track modifier state locally so focus loss can release anything still held. + self.modifiers.update(vk_code, down); + + // The winit path has no hardware scan code; the agent derives one from the VK. + // The extended-key flag is derived from the VK so extended keys (arrows, etc.) + // still inject correctly without a captured LLKHF_EXTENDED bit. let event = proto::KeyEvent { - down: state == ElementState::Pressed, + down, key_type: proto::KeyEventType::KeyVk as i32, vk_code, scan_code: 0, unicode: String::new(), + is_extended: crate::input::vk_is_extended(vk_code as u16), modifiers: Some(proto::Modifiers::default()), }; let _ = self.input_tx.try_send(InputEvent::Key(event)); } + /// Release every modifier this viewer currently believes is held on the remote. + /// + /// Invoked on focus loss and at window close so that a Ctrl/Alt/Shift/Win whose + /// key-up the viewer never saw (because focus left mid-press) is explicitly released + /// on the remote, preventing a "stuck modifier". + fn release_held_modifiers(&mut self) { + for vk in self.modifiers.drain_held() { + let event = proto::KeyEvent { + down: false, + key_type: proto::KeyEventType::KeyVk as i32, + vk_code: vk as u32, + scan_code: 0, + unicode: String::new(), + is_extended: crate::input::vk_is_extended(vk), + modifiers: Some(proto::Modifiers::default()), + }; + let _ = self.input_tx.try_send(InputEvent::Key(event)); + } + } + fn screen_to_frame_coords(&self, x: f64, y: f64) -> (i32, i32) { let Some(window) = &self.window else { return (x as i32, y as i32); @@ -318,6 +395,8 @@ impl ApplicationHandler for ViewerApp { match event { WindowEvent::CloseRequested => { info!("Window close requested"); + // Release any modifiers still held so the remote isn't left latched. + self.release_held_modifiers(); event_loop.exit(); } WindowEvent::RedrawRequested => { @@ -345,9 +424,37 @@ impl ApplicationHandler for ViewerApp { }; self.send_mouse_wheel(dx, dy); } + // Focus changes drive the low-level hook's focus gate. The hook is GLOBAL + // (fires for all desktop input), so it must only divert system keys while the + // viewer is focused; we flip `set_viewer_focused` here. On blur we also release + // any held modifiers so they don't stay latched on the remote — winit's hook + // pump only runs while we have focus, so this is the safety net for a modifier + // pressed-but-not-released across the blur. + WindowEvent::Focused(focused) => { + input::set_viewer_focused(focused); + if focused { + debug!("Viewer gained focus; system-key forwarding active"); + } else { + debug!("Viewer lost focus; releasing held modifiers on remote"); + self.release_held_modifiers(); + } + } // Note: This handles keys that aren't captured by the low-level hook. - // The hook handles Win key and other special keys. + // The hook handles the Windows key and other diverted system combinations. WindowEvent::KeyboardInput { event, .. } if !event.repeat => { + // Host key: Pause/Break toggles "send system keys to remote". It is + // intercepted locally (not forwarded) so the technician can flip the + // behavior without affecting the remote. Only act on key-down. + if matches!(event.physical_key, PhysicalKey::Code(KeyCode::Pause)) + && event.state == ElementState::Pressed + { + let enabled = input::toggle_send_system_keys(); + info!( + "Send-system-keys toggled {} (Pause/Break host key)", + if enabled { "ON" } else { "OFF" } + ); + return; + } self.send_key_event(event.physical_key, event.state); } _ => {} diff --git a/proto/guruconnect.proto b/proto/guruconnect.proto index bcc5e4f..1a2fbdb 100644 --- a/proto/guruconnect.proto +++ b/proto/guruconnect.proto @@ -148,6 +148,13 @@ message KeyEvent { uint32 scan_code = 4; // Hardware scan code string unicode = 5; // Unicode character (for text input) Modifiers modifiers = 6; + // True when the originating key is an "extended" key (right Ctrl/Alt, arrows, + // Insert/Home/End/PageUp/Down, NumLock, Win keys, numpad Divide). The viewer + // captures this from the low-level hook (KBDLLHOOKSTRUCT.flags & LLKHF_EXTENDED); + // the agent injects with KEYEVENTF_EXTENDEDKEY when set. Field added in Task 6 + // (v2 full key fidelity); older agents that ignore it fall back to deriving the + // flag from vk_code/scan_code. + bool is_extended = 7; } enum KeyEventType { diff --git a/specs/v2-secure-session-core/plan.md b/specs/v2-secure-session-core/plan.md index b2a9325..a9ba7c3 100644 --- a/specs/v2-secure-session-core/plan.md +++ b/specs/v2-secure-session-core/plan.md @@ -316,11 +316,69 @@ Reference: `specs/native-remote-control/plan.md` Task 5 (Consent primitive); pro --- -## Task 6: Native viewer — full key fidelity +## Task 6 [IMPLEMENTED 2026-05-30 — self-verified on a local Windows toolchain: `cargo fmt --all` clean, `cargo clippy --workspace --all-targets --all-features -- -D warnings` exit 0, `cargo test --workspace` 69 pass (19 agent + 50 server), `cargo build --workspace` ok; pending Code Review]: Native viewer — full key fidelity -Files touched: `agent/src/viewer/` (low-level hook + input capture), `agent/src/input/keyboard.rs` -(extend — salvaged), `agent/src/input/mod.rs`, `agent/src/bin/sas_service.rs` (wire — salvaged), -`proto/guruconnect.proto` (confirm `KeyEvent`/`SpecialKeyEvent` coverage). +> [IMPLEMENTED] The four parts: +> +> 1. VIEWER CAPTURE (`agent/src/viewer/input.rs`): the `WH_KEYBOARD_LL` hook now +> DIVERTS system combinations (Windows/Apps keys, Alt+Tab, Alt+Esc, Ctrl+Esc; +> Win+R / Win+E compose on the remote because the diverted Win-down is forwarded) +> and forwards them as full-fidelity `KeyEvent`s — VK + hardware scan code + +> `is_extended` (read from `KBDLLHOOKSTRUCT.flags & LLKHF_EXTENDED`) + modifier +> snapshot — returning `LRESULT(1)` to suppress local handling. The combo decision +> is a pure `is_system_combo(vk, alt, ctrl)` so it is unit-tested. Ordinary keys are +> NOT forwarded by the hook (they take the normal winit path — avoids double inject). +> A "send system keys to remote" toggle (`AtomicBool`, default ON) gates the diversion; +> when OFF the hook is transparent. Toggle API: `set_/toggle_/send_system_keys_enabled`. +> Host key: Pause/Break flips it (handled in `render.rs`, intercepted locally, logged). +> 2. AGENT INJECTION (`agent/src/input/keyboard.rs`): rewrote `send_key` to inject via +> `SendInput` with `KEYEVENTF_SCANCODE` (layout-independent) and the correct +> `KEYEVENTF_EXTENDEDKEY` (set when the viewer flagged extended, the VK is inherently +> extended, or the VK→scan map carries the 0xE0 prefix). New `key_event_full(vk, scan, +> is_extended, down)` consumes all `KeyEvent` fields (scan 0 ⇒ derive from VK for older +> viewers); wired into `session/mod.rs` `KeyEvent` handling. `is_extended_key` delegates +> to a platform-independent `vk_is_extended` (unit-tested) that also covers right Ctrl/Alt. +> 3. CTRL+ALT+DEL: the existing `SpecialKey::CtrlAltDel` → `send_ctrl_alt_del` → +> `send_sas` path is confirmed and hardened. `send_sas` now: Tier 1 SAS helper service +> (SYSTEM, named pipe) → Tier 2 direct `sas.dll!SendSAS` → Tier 3 **fails with a clear, +> actionable error** (no false success; plain SendInput can't reach the secure desktop). +> The SAS installer (`bin/sas_service.rs install`) now sets the `SoftwareSASGeneration` +> Winlogon policy (HKLM\...\Policies\System, DWORD 1 = services) via `set_software_sas_policy`, +> with a `// TODO(installer)` noting the top-level managed installer should also ensure it. +> 4. MODIFIER HYGIENE: viewer tracks held Ctrl/Alt/Shift/Win (`ViewerModifierState` in +> `render.rs`) and emits explicit key-ups on FOCUS LOSS (`WindowEvent::Focused(false)`) +> and on window close, so a modifier pressed-but-not-released across a blur doesn't stay +> latched on the remote. Agent-side `KeyboardController` also tracks injected modifiers and +> exposes `release_all_modifiers` (defensive complement; wired via `ModifierState`, the +> scaffolding the cleanup kept). Both trackers unit-tested. +> +> PROTO: `KeyEvent` gained `bool is_extended = 7` (new field number, nothing renumbered) +> — carries the viewer's `LLKHF_EXTENDED` capture so injection picks the right extended +> flag; older agents that ignore it fall back to deriving from vk/scan. `SpecialKeyEvent` +> already carried `CTRL_ALT_DEL`; unchanged. +> +> dead_code wired/removed: `InputEvent::SpecialKey` (now emitted/consumable), +> `ModifierState` (agent keyboard — now tracks + drains held modifiers), +> `type_char`/`type_string` kept with their allows (separate unicode-typing feature, not in +> Task 6 scope). `sas_client::is_service_available`/`get_service_status` kept narrowly allowed +> (status API not yet wired into a runtime path). `InputController::key_event` (vk-only) and +> `release_all_modifiers` kept allowed as API surface (the relayed path uses `key_event_full`, +> focus-loss re-sync is viewer-driven). New toggle accessors `set_/send_system_keys_enabled` +> narrowly allowed (host-key uses `toggle_`; future viewer menu). +> +> TESTS ADDED (13): extended-key flag determination (extended + non-extended sets), +> modifier-state transitions (record/down-up/ignore-non-modifier/drain-and-clear), the +> system-combo classifier (Win/Alt+Tab/Alt+Esc/Ctrl+Esc divert; ordinary keys don't), and +> the toggle state machine (default ON, flip, explicit set). Live hook/SendInput/SendSAS +> behavior is plan Task 8 (needs a real desktop). + +Files touched: `proto/guruconnect.proto` (KeyEvent `is_extended = 7`), `agent/src/viewer/input.rs` +(rewritten hook + toggle), `agent/src/viewer/render.rs` (focus-loss modifier release, host-key +toggle, is_extended on winit path), `agent/src/viewer/mod.rs` (unchanged SpecialKey forwarding), +`agent/src/input/keyboard.rs` (scan-code injection + ModifierState + vk_is_extended + tests), +`agent/src/input/mod.rs` (key_event_full / release_all_modifiers / vk_is_extended re-export), +`agent/src/session/mod.rs` (KeyEvent → key_event_full), `agent/src/bin/sas_service.rs` +(set_software_sas_policy + install wiring), `agent/src/input/keyboard.rs` (send_sas hardening). - **Viewer capture:** install `WH_KEYBOARD_LL` while the viewer is in focused control; divert system combos (Win key, Win+R, Alt+Tab, Ctrl+Esc) and forward as `KeyEvent` (VK + scan code + extended flag)