[PM-27646] Prevent enabling logging and disabling signature (#17253)

This ensures we won't accidentally ship prod binaries that either have logging enabled or disable signature validation.

apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/config.rs

@@ -1,10 +1,2 @@
-// Enable this to log to a file. The way this executable is used, it's not easy to debug and the stdout gets lost.
-// This is intended for development time only.
-pub(crate) const ENABLE_DEVELOPER_LOGGING: bool = false;
-pub(crate) const LOG_FILENAME: &str = "c:\\path\\to\\log.txt"; // This is an example filename, replace it with you own
-
-// This should be enabled for production
-pub(crate) const ENABLE_SERVER_SIGNATURE_VALIDATION: bool = true;
-
 // List of SYSTEM process names to try to impersonate
 pub(crate) const SYSTEM_PROCESS_NAMES: [&str; 2] = ["services.exe", "winlogon.exe"];

apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/log.rs

@@ -3,7 +3,7 @@ use tracing_subscriber::{
     fmt, layer::SubscriberExt as _, util::SubscriberInitExt as _, EnvFilter, Layer as _,
 };
 
-use super::config::{ENABLE_DEVELOPER_LOGGING, LOG_FILENAME};
+use chromium_importer::config::{ENABLE_DEVELOPER_LOGGING, LOG_FILENAME};
 
 pub(crate) fn init_logging() {
     if ENABLE_DEVELOPER_LOGGING {

apps/desktop/desktop_native/bitwarden_chromium_import_helper/src/windows/main.rs

@@ -28,7 +28,6 @@ use windows::Win32::{
 use chromium_importer::chromium::{verify_signature, ADMIN_TO_USER_PIPE_NAME};
 
 use super::{
-    config::ENABLE_SERVER_SIGNATURE_VALIDATION,
     crypto::{
         decode_abe_key_blob, decode_base64, decrypt_with_dpapi_as_system,
         decrypt_with_dpapi_as_user, encode_base64,
@@ -144,22 +143,20 @@ fn is_admin() -> bool {
 async fn open_and_validate_pipe_server(pipe_name: &'static str) -> Result<NamedPipeClient> {
     let client = open_pipe_client(pipe_name).await?;
 
-    if ENABLE_SERVER_SIGNATURE_VALIDATION {
+    let server_pid = get_named_pipe_server_pid(&client)?;
-        let server_pid = get_named_pipe_server_pid(&client)?;
+    debug!("Connected to pipe server PID {}", server_pid);
-        debug!("Connected to pipe server PID {}", server_pid);
 
-        // Validate the server end process signature
+    // Validate the server end process signature
-        let exe_path = resolve_process_executable_path(server_pid)?;
+    let exe_path = resolve_process_executable_path(server_pid)?;
 
-        debug!("Pipe server executable path: {}", exe_path.display());
+    debug!("Pipe server executable path: {}", exe_path.display());
 
-        if !verify_signature(&exe_path)? {
+    if !verify_signature(&exe_path)? {
-            return Err(anyhow!("Pipe server signature is not valid"));
+        return Err(anyhow!("Pipe server signature is not valid"));
-        }
-
-        debug!("Pipe server signature verified for PID {}", server_pid);
     }
 
+    debug!("Pipe server signature verified for PID {}", server_pid);
+
     Ok(client)
 }
 

apps/desktop/desktop_native/chromium_importer/build.rs

@@ -0,0 +1,15 @@
+include!("config_constants.rs");
+
+fn main() {
+    println!("cargo:rerun-if-changed=config_constants.rs");
+
+    if cfg!(not(debug_assertions)) {
+        if ENABLE_DEVELOPER_LOGGING {
+            panic!("ENABLE_DEVELOPER_LOGGING must be false in release builds");
+        }
+
+        if !ENABLE_SIGNATURE_VALIDATION {
+            panic!("ENABLE_SIGNATURE_VALIDATION must be true in release builds");
+        }
+    }
+}

apps/desktop/desktop_native/chromium_importer/config_constants.rs

@@ -0,0 +1,12 @@
+// Enable this to log to a file. The way this executable is used, it's not easy to debug and the stdout gets lost.
+// This is intended for development time only.
+pub const ENABLE_DEVELOPER_LOGGING: bool = false;
+
+// The absolute path to log file when developer logging is enabled
+// Change this to a suitable path for your environment
+pub const LOG_FILENAME: &str = "c:\\path\\to\\log.txt";
+
+/// Ensure the signature of the helper and main binary is validated in production builds
+///
+/// This must be true in release builds but may be disabled in debug builds for testing.
+pub const ENABLE_SIGNATURE_VALIDATION: bool = true;

apps/desktop/desktop_native/chromium_importer/src/chromium/mod.rs

@@ -10,9 +10,7 @@ use rusqlite::{params, Connection};
 mod platform;
 
 #[cfg(target_os = "windows")]
-pub use platform::{
+pub use platform::{verify_signature, ADMIN_TO_USER_PIPE_NAME};
-    verify_signature, ADMIN_TO_USER_PIPE_NAME, EXPECTED_SIGNATURE_SHA256_THUMBPRINT,
-};
 
 pub(crate) use platform::SUPPORTED_BROWSERS as PLATFORM_SUPPORTED_BROWSERS;
 

apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/mod.rs

@@ -61,9 +61,6 @@ 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
 //
@@ -184,7 +181,7 @@ impl WindowsCryptoService {
 
         let admin_exe_path = get_admin_exe_path()?;
 
-        if ENABLE_SIGNATURE_VALIDATION && !verify_signature(&admin_exe_path)? {
+        if !verify_signature(&admin_exe_path)? {
             return Err(anyhow!("Helper executable signature is not valid"));
         }
 

apps/desktop/desktop_native/chromium_importer/src/chromium/platform/windows/signature.rs

@@ -3,10 +3,20 @@ use std::path::Path;
 use tracing::{debug, info};
 use verifysign::CodeSignVerifier;
 
+use crate::config::ENABLE_SIGNATURE_VALIDATION;
+
 pub const EXPECTED_SIGNATURE_SHA256_THUMBPRINT: &str =
     "9f6680c4720dbf66d1cb8ed6e328f58e42523badc60d138c7a04e63af14ea40d";
 
 pub fn verify_signature(path: &Path) -> Result<bool> {
+    if !ENABLE_SIGNATURE_VALIDATION {
+        info!(
+            "Signature validation is disabled. Skipping verification for: {}",
+            path.display()
+        );
+        return Ok(true);
+    }
+
     info!("verifying signature of: {}", path.display());
 
     let verifier = CodeSignVerifier::for_file(path)

apps/desktop/desktop_native/chromium_importer/src/lib.rs

@@ -1,5 +1,9 @@
 #![doc = include_str!("../README.md")]
 
+pub mod config {
+    include!("../config_constants.rs");
+}
+
 pub mod chromium;
 pub mod metadata;
 mod util;