diff --git a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/Cargo.toml b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/Cargo.toml index d1e4ac899bd..dc5358b0c73 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/Cargo.toml +++ b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/Cargo.toml @@ -12,7 +12,6 @@ chromium_importer = { path = "../chromium_importer" } clap = { version = "=4.5.40", features = ["derive"] } scopeguard = { workspace = true } sysinfo = { workspace = true } -verifysign = "=0.2.4" windows = { workspace = true, features = [ "Wdk_System_SystemServices", "Win32_Security_Cryptography", diff --git a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows.rs b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows.rs index 7ab80ff0f35..9abc8c95a1f 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows.rs @@ -20,7 +20,6 @@ mod windows_binary { use tracing_subscriber::{ fmt, layer::SubscriberExt as _, util::SubscriberInitExt as _, EnvFilter, Layer as _, }; - use verifysign::CodeSignVerifier; use windows::{ core::BOOL, Wdk::System::SystemServices::SE_DEBUG_PRIVILEGE, @@ -45,7 +44,7 @@ mod windows_binary { }, }; - use chromium_importer::chromium::ADMIN_TO_USER_PIPE_NAME; + use chromium_importer::chromium::{verify_signature, ADMIN_TO_USER_PIPE_NAME}; #[derive(Parser)] #[command(name = "bitwarden_chromium_import_helper")] @@ -66,8 +65,6 @@ mod windows_binary { // This should be enabled for production const ENABLE_SERVER_SIGNATURE_VALIDATION: bool = true; - const EXPECTED_SERVER_SIGNATURE_SHA256_THUMBPRINT: &str = - "9f6680c4720dbf66d1cb8ed6e328f58e42523badc60d138c7a04e63af14ea40d"; // List of SYSTEM process names to try to impersonate const SYSTEM_PROCESS_NAMES: [&str; 2] = ["services.exe", "winlogon.exe"]; @@ -383,29 +380,7 @@ mod windows_binary { dbg_log!("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 - ) - })?; - - dbg_log!("Pipe server executable path: {}", exe_path.display()); - - // Dump signature fields for debugging/inspection - dbg_log!("Signature fields:"); - dbg_log!(" Subject Name: {:?}", signature.subject_name()); - dbg_log!(" Issuer Name: {:?}", signature.issuer_name()); - dbg_log!(" SHA1 Thumbprint: {:?}", signature.sha1_thumbprint()); - dbg_log!(" SHA256 Thumbprint: {:?}", signature.sha256_thumbprint()); - dbg_log!(" Serial Number: {:?}", signature.serial()); - - if signature.sha256_thumbprint() != EXPECTED_SERVER_SIGNATURE_SHA256_THUMBPRINT { + if !verify_signature(&exe_path)? { return Err(anyhow!("Pipe server signature is not valid")); } diff --git a/apps/desktop/desktop_native/chromium_importer/Cargo.toml b/apps/desktop/desktop_native/chromium_importer/Cargo.toml index 99246af3f90..51ad450a6fc 100644 --- a/apps/desktop/desktop_native/chromium_importer/Cargo.toml +++ b/apps/desktop/desktop_native/chromium_importer/Cargo.toml @@ -43,6 +43,7 @@ windows = { workspace = true, features = [ "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging", ] } +verifysign = "=0.2.4" [target.'cfg(target_os = "linux")'.dependencies] oo7 = { workspace = true } diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs index 0873c66d053..471e35da23e 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs @@ -10,7 +10,9 @@ use rusqlite::{params, Connection}; mod platform; #[cfg(target_os = "windows")] -pub use platform::ADMIN_TO_USER_PIPE_NAME; +pub use platform::{ + verify_signature, ADMIN_TO_USER_PIPE_NAME, EXPECTED_SIGNATURE_SHA256_THUMBPRINT, +}; pub(crate) use platform::SUPPORTED_BROWSERS as PLATFORM_SUPPORTED_BROWSERS; diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs index e47e9bf08d2..a8045cf1182 100644 --- a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs @@ -13,8 +13,10 @@ use crate::chromium::{BrowserConfig, CryptoService, LocalState}; use crate::util; mod abe; mod abe_config; +mod signature; pub use abe_config::ADMIN_TO_USER_PIPE_NAME; +pub use signature::*; // // Public API @@ -60,6 +62,9 @@ pub(crate) fn get_crypto_service( const ADMIN_EXE_FILENAME: &str = "bitwarden_chromium_import_helper.exe"; +// This should be enabled for production +const ENABLE_SIGNATURE_VALIDATION: bool = true; + // // CryptoService // @@ -179,6 +184,11 @@ impl WindowsCryptoService { } let admin_exe_path = get_admin_exe_path()?; + + if ENABLE_SIGNATURE_VALIDATION && !verify_signature(&admin_exe_path)? { + return Err(anyhow!("Helper executable signature is not valid")); + } + let admin_exe_str = admin_exe_path .to_str() .ok_or_else(|| anyhow!("Failed to convert {} path to string", ADMIN_EXE_FILENAME))?; diff --git a/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/signature.rs b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/signature.rs new file mode 100644 index 00000000000..a30b396db28 --- /dev/null +++ b/apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/signature.rs @@ -0,0 +1,28 @@ +use anyhow::{anyhow, Result}; +use std::path::Path; +use tracing::{debug, info}; +use verifysign::CodeSignVerifier; + +pub const EXPECTED_SIGNATURE_SHA256_THUMBPRINT: &str = + "9f6680c4720dbf66d1cb8ed6e328f58e42523badc60d138c7a04e63af14ea40d"; + +pub fn verify_signature(path: &Path) -> Result { + info!("verifying signature of: {}", path.display()); + + let verifier = CodeSignVerifier::for_file(path) + .map_err(|e| anyhow!("verifysign init failed for {}: {:?}", path.display(), e))?; + + let signature = verifier + .verify() + .map_err(|e| anyhow!("verifysign verify failed for {}: {:?}", path.display(), e))?; + + // 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()); + + Ok(signature.sha256_thumbprint() == EXPECTED_SIGNATURE_SHA256_THUMBPRINT) +}