From db39bb1dcc7bfd7abd01e7a361f52afb1bb7c72c Mon Sep 17 00:00:00 2001 From: Dmitry Yakimenko Date: Tue, 28 Oct 2025 13:29:05 +0100 Subject: [PATCH 1/2] Address PR change requests. Mainly small cleanup and renaming. --- .../bitwarden_chromium_importer/Cargo.toml | 6 +-- .../bitwarden_chromium_importer/src/abe.rs | 4 +- .../src/abe_config.rs | 3 +- .../bin/bitwarden_chromium_import_helper.rs | 41 ++++++++++--------- apps/desktop/desktop_native/build.js | 21 +++++----- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml b/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml index 2f3ca776138..5e4cf22c737 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/Cargo.toml @@ -23,16 +23,16 @@ rusqlite = { version = "=0.37.0", features = ["bundled"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha1 = "=0.10.6" -sysinfo = { workspace = true, optional = true } [target.'cfg(target_os = "macos")'.dependencies] security-framework = { workspace = true } [target.'cfg(target_os = "windows")'.dependencies] -chacha20poly1305 = "=0.10.1" +chacha20poly1305 = { workspace = true } clap = { version = "=4.5.40", features = ["derive"] } scopeguard = { workspace = true } simplelog = { workspace = true } +sysinfo = { workspace = true, optional = true } tokio = { workspace = true, features = ["full"] } verifysign = "=0.2.4" windows = { workspace = true, features = [ @@ -57,7 +57,7 @@ oo7 = { workspace = true } workspace = true [features] -windows-binary = ["sysinfo"] +windows-binary = ["dep:sysinfo"] [[bin]] name = "bitwarden_chromium_import_helper" diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe.rs index 9081a86188e..2e0d62a60fc 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe.rs @@ -17,7 +17,7 @@ use crate::abe_config; const WAIT_FOR_ADMIN_MESSAGE_TIMEOUT_SECS: u64 = 30; -pub fn start_tokio_named_pipe_server( +fn start_tokio_named_pipe_server( pipe_name: &'static str, process_message: F, ) -> Result>> @@ -111,7 +111,7 @@ where } } -pub async fn decrypt_with_admin_exe(admin_exe: &str, encrypted: &str) -> Result { +pub(crate) async fn decrypt_with_admin_exe(admin_exe: &str, encrypted: &str) -> Result { let (tx, mut rx) = channel::(1); debug!( diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe_config.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe_config.rs index a1c36d700a8..66b1d3b8435 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe_config.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/abe_config.rs @@ -1 +1,2 @@ -pub const ADMIN_TO_USER_PIPE_NAME: &str = r"\\.\pipe\BitwardenEncryptionService-admin-user"; +pub const ADMIN_TO_USER_PIPE_NAME: &str = + r"\\.\pipe\bitwarden-to-bitwarden-chromium-importer-helper"; diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs index 26d317e52c0..8b5742bca9b 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs @@ -144,20 +144,23 @@ mod windows_binary { _ = CloseHandle(hprocess); }); - let mut wide = vec![0u16; 260]; - let mut size = wide.len() as u32; + let mut exe_name = vec![0u16; 32 * 1024]; + let mut exe_name_length = exe_name.len() as u32; unsafe { QueryFullProcessImageNameW( hprocess, PROCESS_NAME_WIN32, - windows::core::PWSTR(wide.as_mut_ptr()), - &mut size, + windows::core::PWSTR(exe_name.as_mut_ptr()), + &mut exe_name_length, ) }?; - dbg_log!("QueryFullProcessImageNameW returned {} bytes", size); + dbg_log!( + "QueryFullProcessImageNameW returned {} bytes", + exe_name_length + ); - wide.truncate(size as usize); - Ok(PathBuf::from(OsString::from_wide(&wide))) + exe_name.truncate(exe_name_length as usize); + Ok(PathBuf::from(OsString::from_wide(&exe_name))) } async fn send_error_to_user(client: &mut NamedPipeClient, error_message: &str) { @@ -208,7 +211,7 @@ mod windows_binary { let result = unsafe { CryptUnprotectData( &in_blob, - Some(ptr::null_mut()), + None, None, None, None, @@ -248,7 +251,7 @@ mod windows_binary { } impl ImpersonateGuard { - pub fn start() -> Result { + fn start() -> Result { Self::enable_privilege()?; // Find a SYSTEM process and get its token. Not every SYSTEM process allows token duplication, so try several. @@ -266,7 +269,7 @@ mod windows_binary { }) } - pub fn stop() -> Result<()> { + fn stop() -> Result<()> { unsafe { RevertToSelf()?; }; @@ -274,12 +277,12 @@ mod windows_binary { } /// stop impersonate and return sys token handle - pub fn _stop_sys_handle(self) -> Result { + fn _stop_sys_handle(self) -> Result { unsafe { RevertToSelf() }?; Ok(self.sys_token_handle) } - pub fn close_sys_handle(&self) -> Result<()> { + fn close_sys_handle(&self) -> Result<()> { unsafe { CloseHandle(self.sys_token_handle) }?; Ok(()) } @@ -351,14 +354,14 @@ mod windows_binary { } fn get_system_pid_list() -> Vec<(u32, &'static str)> { - let mut pids = Vec::new(); let sys = System::new_all(); - for name in SYSTEM_PROCESS_NAMES { - for process in sys.processes_by_exact_name(name.as_ref()) { - pids.push((process.pid().as_u32(), name)); - } - } - pids + SYSTEM_PROCESS_NAMES + .iter() + .flat_map(|&name| { + sys.processes_by_exact_name(name.as_ref()) + .map(move |process| (process.pid().as_u32(), name)) + }) + .collect() } fn get_process_handle(pid: u32) -> Result { diff --git a/apps/desktop/desktop_native/build.js b/apps/desktop/desktop_native/build.js index 5df033a1634..a5a73dfadef 100644 --- a/apps/desktop/desktop_native/build.js +++ b/apps/desktop/desktop_native/build.js @@ -51,18 +51,17 @@ function buildImporterBinaries(target, release = true) { return; } - ["bitwarden_chromium_import_helper"].forEach(bin => { - const targetArg = target ? `--target ${target}` : ""; - const releaseArg = release ? "--release" : ""; - child_process.execSync(`cargo build --bin ${bin} ${releaseArg} ${targetArg} --features windows-binary`, {stdio: 'inherit', cwd: path.join(__dirname, "bitwarden_chromium_importer")}); + const bin = "bitwarden_chromium_import_helper"; + const targetArg = target ? `--target ${target}` : ""; + const releaseArg = release ? "--release" : ""; + child_process.execSync(`cargo build --bin ${bin} ${releaseArg} ${targetArg} --features windows-binary`, {stdio: 'inherit', cwd: path.join(__dirname, "bitwarden_chromium_importer")}); - if (target) { - // Copy the resulting binary to the dist folder - const targetFolder = release ? "release" : "debug"; - const nodeArch = rustTargetsMap[target].nodeArch; - fs.copyFileSync(path.join(__dirname, "target", target, targetFolder, `${bin}.exe`), path.join(__dirname, "dist", `${bin}.${process.platform}-${nodeArch}.exe`)); - } - }); + if (target) { + // Copy the resulting binary to the dist folder + const targetFolder = release ? "release" : "debug"; + const nodeArch = rustTargetsMap[target].nodeArch; + fs.copyFileSync(path.join(__dirname, "target", target, targetFolder, `${bin}.exe`), path.join(__dirname, "dist", `${bin}.${process.platform}-${nodeArch}.exe`)); + } } function buildProcessIsolation() { From c619b2607df993bd5946dbd3dc91cc569a9c113f Mon Sep 17 00:00:00 2001 From: Dmitry Yakimenko Date: Tue, 28 Oct 2025 13:42:21 +0100 Subject: [PATCH 2/2] Don't loop forever, but retry a few times --- .../src/bin/bitwarden_chromium_import_helper.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs index 8b5742bca9b..f84cecf6465 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/bin/bitwarden_chromium_import_helper.rs @@ -82,12 +82,12 @@ mod windows_binary { } async fn open_pipe_client(pipe_name: &'static str) -> Result { - // TODO: Don't loop forever, but retry a few times - let client = loop { + let max_attempts = 5; + for _ in 0..max_attempts { match ClientOptions::new().open(pipe_name) { Ok(client) => { dbg_log!("Successfully connected to the pipe!"); - break client; + return Ok(client); } Err(e) if e.raw_os_error() == Some(ERROR_PIPE_BUSY.0 as i32) => { dbg_log!("Pipe is busy, retrying in 50ms..."); @@ -99,9 +99,12 @@ mod windows_binary { } time::sleep(Duration::from_millis(50)).await; - }; + } - Ok(client) + Err(anyhow!( + "Failed to connect to pipe after {} attempts", + max_attempts + )) } async fn send_message_with_client(