1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00

[PM-27645] Check signature of helper exe (#17155)

This commit is contained in:
Oscar Hinton
2025-10-31 16:50:13 +01:00
committed by GitHub
parent 443b85a356
commit 8c185c9d2b
6 changed files with 44 additions and 29 deletions

View File

@@ -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",

View File

@@ -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"));
}

View File

@@ -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 }

View File

@@ -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;

View File

@@ -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))?;

View File

@@ -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<bool> {
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)
}