From 86b43d80ce0f5fb9c90817bb1b7c39e14ba34f86 Mon Sep 17 00:00:00 2001 From: Dmitry Yakimenko Date: Wed, 1 Oct 2025 20:45:50 +0200 Subject: [PATCH] - Verify the signature of the server process on the other end of the pipe (PoC) - Improved error handling - More logging --- apps/desktop/desktop_native/Cargo.lock | 49 ++++- .../bitwarden_chromium_importer/Cargo.toml | 2 + .../src/bin/admin.rs | 197 +++++++++++++----- 3 files changed, 185 insertions(+), 63 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 92e6f25315e..b99cb55c0b5 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -455,12 +455,14 @@ dependencies = [ "pbkdf2", "rand 0.9.1", "rusqlite", + "scopeguard", "security-framework", "serde", "serde_json", "sha1", "simplelog", "tokio", + "verifysign", "windows 0.61.1", ] @@ -1025,7 +1027,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys 0.60.2", ] [[package]] @@ -1726,7 +1728,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a793df0d7afeac54f95b471d3af7f0d4fb975699f972341a4b76988d49cdf0c" dependencies = [ "cfg-if", - "windows-targets 0.52.6", + "windows-targets 0.53.3", ] [[package]] @@ -3632,6 +3634,18 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" +[[package]] +name = "verifysign" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ebfe12e38930c3b851aea35e93fab1a6c29279cad7e8e273f29a21678fee8c0" +dependencies = [ + "core-foundation", + "sha1", + "sha2", + "windows-sys 0.61.1", +] + [[package]] name = "version_check" version = "0.9.5" @@ -3788,7 +3802,7 @@ dependencies = [ "windows-collections", "windows-core 0.61.0", "windows-future", - "windows-link", + "windows-link 0.1.3", "windows-numerics", ] @@ -3821,7 +3835,7 @@ checksum = "4763c1de310c86d75a878046489e2e5ba02c649d185f21c67d4cf8a56d098980" dependencies = [ "windows-implement 0.60.0", "windows-interface 0.59.1", - "windows-link", + "windows-link 0.1.3", "windows-result 0.3.4", "windows-strings", ] @@ -3833,7 +3847,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a1d6bbefcb7b60acd19828e1bc965da6fcf18a7e39490c5f8be71e54a19ba32" dependencies = [ "windows-core 0.61.0", - "windows-link", + "windows-link 0.1.3", ] [[package]] @@ -3886,6 +3900,12 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" +[[package]] +name = "windows-link" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45e46c0661abb7180e7b9c281db115305d49ca1709ab8242adf09666d2173c65" + [[package]] name = "windows-numerics" version = "0.2.0" @@ -3893,7 +3913,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" dependencies = [ "windows-core 0.61.0", - "windows-link", + "windows-link 0.1.3", ] [[package]] @@ -3902,7 +3922,7 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b8a9ed28765efc97bbc954883f4e6796c33a06546ebafacbabee9696967499e" dependencies = [ - "windows-link", + "windows-link 0.1.3", "windows-result 0.3.4", "windows-strings", ] @@ -3922,7 +3942,7 @@ version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" dependencies = [ - "windows-link", + "windows-link 0.1.3", ] [[package]] @@ -3931,7 +3951,7 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" dependencies = [ - "windows-link", + "windows-link 0.1.3", ] [[package]] @@ -3961,6 +3981,15 @@ dependencies = [ "windows-targets 0.53.3", ] +[[package]] +name = "windows-sys" +version = "0.61.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f109e41dd4a3c848907eb83d5a42ea98b3769495597450cf6d153507b166f0f" +dependencies = [ + "windows-link 0.2.0", +] + [[package]] name = "windows-targets" version = "0.48.5" @@ -3998,7 +4027,7 @@ version = "0.53.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d5fe6031c4041849d7c496a8ded650796e7b6ecc19df1a431c1a363342e5dc91" dependencies = [ - "windows-link", + "windows-link 0.1.3", "windows_aarch64_gnullvm 0.53.0", "windows_aarch64_msvc 0.53.0", "windows_i686_gnu 0.53.0", diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml b/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml index 4396e9efb52..eda67ffa90a 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml @@ -28,8 +28,10 @@ security-framework = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] chacha20poly1305 = "=0.10.1" clap = { version = "=4.5.40", features = ["derive"] } +scopeguard = { workspace = true } simplelog = { workspace = true } tokio = { workspace = true, features = ["full"] } +verifysign = "=0.2.4" windows = { workspace = true, features = [ "Wdk_System_SystemServices", "Win32_Security_Cryptography", diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/admin.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/admin.rs index 205e381e030..d5fddd8aa38 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/admin.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/admin.rs @@ -1,21 +1,23 @@ use anyhow::{anyhow, Result}; use base64::{engine::general_purpose, Engine as _}; use clap::Parser; -use log::debug; +use log::{debug, error}; +use scopeguard::guard; use simplelog::*; use std::{ ffi::{OsStr, OsString}, fs::OpenOptions, - os::windows::ffi::OsStringExt as _, + os::windows::{ffi::OsStringExt as _, io::AsRawHandle}, path::PathBuf, ptr, time::Duration, }; use tokio::{ io::{AsyncReadExt, AsyncWriteExt}, - net::windows::named_pipe::ClientOptions, + net::windows::named_pipe::{ClientOptions, NamedPipeClient}, time, }; +use verifysign::CodeSignVerifier; use windows::{ core::BOOL, Wdk::System::SystemServices::SE_DEBUG_PRIVILEGE, @@ -29,9 +31,11 @@ use windows::{ DuplicateToken, ImpersonateLoggedOnUser, RevertToSelf, TOKEN_DUPLICATE, TOKEN_QUERY, }, System::{ + Pipes::GetNamedPipeServerProcessId, ProcessStatus::{EnumProcesses, K32GetProcessImageFileNameW}, Threading::{ - OpenProcess, OpenProcessToken, PROCESS_QUERY_INFORMATION, PROCESS_VM_READ, + OpenProcess, OpenProcessToken, QueryFullProcessImageNameW, PROCESS_NAME_WIN32, + PROCESS_QUERY_INFORMATION, PROCESS_VM_READ, }, }, UI::Shell::IsUserAnAdmin, @@ -53,9 +57,14 @@ struct Args { const NEED_LOGGING: bool = false; const LOG_FILENAME: &str = "c:\\path\\to\\log.txt"; // This is an example filename, replace it with you own -async fn send_message_to_pipe_server(pipe_name: &'static str, message: &str) -> Result { +// This should be enabled for production +const NEED_SERVER_SIGNATURE_VALIDATION: bool = false; +const EXPECTED_SERVER_SIGNATURE_SHA256_THUMBPRINT: &str = + "9f6680c4720dbf66d1cb8ed6e328f58e42523badc60d138c7a04e63af14ea40d"; + +async fn open_pipe_client(pipe_name: &'static str) -> Result { // TODO: Don't loop forever, but retry a few times - let mut client = loop { + let client = loop { match ClientOptions::new().open(pipe_name) { Ok(client) => { debug!("Successfully connected to the pipe!"); @@ -71,8 +80,12 @@ async fn send_message_to_pipe_server(pipe_name: &'static str, message: &str) -> } time::sleep(Duration::from_millis(50)).await; - }; // Send multiple messages to the server + }; + Ok(client) +} + +async fn send_message_with_client(client: &mut NamedPipeClient, message: &str) -> Result { client.write_all(message.as_bytes()).await?; // Try to receive a response for this message @@ -85,18 +98,52 @@ async fn send_message_to_pipe_server(pipe_name: &'static str, message: &str) -> let response = String::from_utf8_lossy(&buffer[..bytes_received]); Ok(response.to_string()) } - Err(e) => { - return Err(anyhow!("Failed to receive response for message: {}", e)); - } + Err(e) => Err(anyhow!("Failed to receive response for message: {}", e)), } } -async fn send_error_to_user(error_message: &str) { - _ = send_to_user(&format!("!{}", error_message)).await +fn get_named_pipe_server_pid(client: &NamedPipeClient) -> Result { + let handle = HANDLE(client.as_raw_handle() as _); + let mut pid: u32 = 0; + unsafe { GetNamedPipeServerProcessId(handle, &mut pid) }?; + Ok(pid) } -async fn send_to_user(message: &str) { - _ = send_message_to_pipe_server(abe_config::ADMIN_TO_USER_PIPE_NAME, &message).await +fn resolve_process_executable_path(pid: u32) -> Result { + debug!("Resolving process executable path for PID {}", pid); + + // Open the process handle + let hprocess = unsafe { OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, false, pid) }?; + debug!("Opened process handle for PID {}", pid); + + let _guard = guard(hprocess, |_| unsafe { + debug!("Closing process handle for PID {}", pid); + _ = CloseHandle(hprocess); + }); + + let mut wide = vec![0u16; 260]; + let mut size = wide.len() as u32; + _ = unsafe { + QueryFullProcessImageNameW( + hprocess, + PROCESS_NAME_WIN32, + windows::core::PWSTR(wide.as_mut_ptr()), + &mut size, + ) + }?; + debug!("QueryFullProcessImageNameW returned {} bytes", size); + + wide.truncate(size as usize); + Ok(PathBuf::from(OsString::from_wide(&wide))) +} + +async fn send_error_to_user(client: &mut NamedPipeClient, error_message: &str) { + _ = send_to_user(client, &format!("!{}", error_message)).await +} + +async fn send_to_user(client: &mut NamedPipeClient, message: &str) -> Result<()> { + let _ = send_message_with_client(client, &message).await?; + Ok(()) } fn is_admin() -> bool { @@ -230,11 +277,16 @@ impl ImpersonateGuard { fn enable_privilege() -> Result<()> { let mut previous_value = BOOL(0); let status = unsafe { + debug!("Setting SE_DEBUG_PRIVILEGE to 1 via RtlAdjustPrivilege"); RtlAdjustPrivilege(SE_DEBUG_PRIVILEGE, BOOL(1), BOOL(0), &mut previous_value) }; if status != STATUS_SUCCESS { return Err(anyhow!("Failed to adjust privilege")); } + debug!( + "SE_DEBUG_PRIVILEGE set to 1, was {} before", + previous_value.0 + ); Ok(()) } @@ -319,14 +371,68 @@ unsafe extern "system" { ) -> NTSTATUS; } -macro_rules! debug_and_send_error { - ($($arg:tt)*) => { - { - let error_message = format!($($arg)*); - debug!("{}", error_message); - send_error_to_user(&error_message).await; +async fn open_and_validate_pipe_server(pipe_name: &'static str) -> Result { + let client = open_pipe_client(pipe_name).await?; + + if NEED_SERVER_SIGNATURE_VALIDATION { + let server_pid = get_named_pipe_server_pid(&client)?; + debug!("Connected to pipe server PID {}", server_pid); + + // Validate the server end process signature + let exe_path = resolve_process_executable_path(server_pid)?; + + debug!("Pipe server executable path: {}", exe_path.display()); + + let verifier = CodeSignVerifier::for_file(exe_path.as_path()) + .map_err(|e| anyhow!("verifysign init failed for {}: {:?}", exe_path.display(), e))?; + + let signature = verifier.verify().map_err(|e| { + anyhow!( + "verifysign verify failed for {}: {:?}", + exe_path.display(), + e + ) + })?; + + debug!("Pipe server executable path: {}", exe_path.display()); + + // Dump signature fields for debugging/inspection + debug!("Signature fields:"); + debug!(" Subject Name: {:?}", signature.subject_name()); + debug!(" Issuer Name: {:?}", signature.issuer_name()); + debug!(" SHA1 Thumbprint: {:?}", signature.sha1_thumbprint()); + debug!(" SHA256 Thumbprint: {:?}", signature.sha256_thumbprint()); + debug!(" Serial Number: {:?}", signature.serial()); + + if signature.sha256_thumbprint() != EXPECTED_SERVER_SIGNATURE_SHA256_THUMBPRINT { + return Err(anyhow!("Pipe server signature is not valid")); } - }; + + debug!("Pipe server signature verified for PID {}", server_pid); + } + + Ok(client) +} + +async fn run() -> Result { + debug!("Starting admin.exe"); + + let args = Args::try_parse()?; + + if !is_admin() { + return Err(anyhow!("Expected to run with admin privileges")); + } + + debug!("Running as admin"); + + // Impersonate a SYSTEM process to be able to decrypt data encrypted for the machine + let (_guard, pid) = ImpersonateGuard::start(None, None)?; + debug!("Impersonating system process with PID {}", pid); + + let system_decrypted_base64 = decrypt_data_base64(&args.encrypted, true)?; + debug!("Decrypted data with system"); + + Ok(system_decrypted_base64) } #[tokio::main] @@ -344,42 +450,27 @@ async fn main() { .expect("Failed to initialize logger"); } - debug!("Starting admin"); - - let args = match Args::try_parse() { - Ok(args) => args, + let mut client = match open_and_validate_pipe_server(abe_config::ADMIN_TO_USER_PIPE_NAME).await + { + Ok(client) => client, Err(e) => { - debug_and_send_error!("Failed to parse command line arguments: {}", e); + error!( + "Failed to open pipe {} to send result/error: {}", + abe_config::ADMIN_TO_USER_PIPE_NAME, + e + ); return; } }; - if !is_admin() { - debug_and_send_error!("Expected to run with admin privileges"); - return; + match run().await { + Ok(system_decrypted_base64) => { + debug!("Sending response back to user"); + let _ = send_to_user(&mut client, &system_decrypted_base64).await; + } + Err(e) => { + debug!("Error: {}", e); + send_error_to_user(&mut client, &format!("{}", e)).await; + } } - - debug!("Running as admin"); - - // Impersonate a SYSTEM process to be able to decrypt data encrypted for the machine - let system_decrypted_base64 = { - // TODO: Handle errors better and report back to the user! - let (_guard, pid) = ImpersonateGuard::start(None, None).unwrap(); - debug!("Impersonating system process with PID {}", pid); - - let system_decrypted = decrypt_data_base64(&args.encrypted, true); - debug!("Decrypted data with system: {:?}", system_decrypted); - - if let Err(e) = system_decrypted { - debug_and_send_error!("Failed to decrypt data: {}", e); - return; - } - - system_decrypted.unwrap() - }; - - debug!("Sending response back to user"); - send_to_user(&system_decrypted_base64).await; - - return; }