From 8090a89a32c7634820bd550df7aaa106f69ecfab Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 5 Aug 2024 21:51:38 +0200 Subject: [PATCH] Add rust module to prevent run-time memory dumping of main (#9393) --- apps/desktop/desktop_native/Cargo.lock | 1 + apps/desktop/desktop_native/core/Cargo.toml | 1 + apps/desktop/desktop_native/core/src/lib.rs | 1 + .../core/src/process_isolation/linux.rs | 51 +++++++++++++++++++ .../core/src/process_isolation/macos.rs | 13 +++++ .../core/src/process_isolation/mod.rs | 5 ++ .../core/src/process_isolation/windows.rs | 13 +++++ apps/desktop/desktop_native/napi/index.d.ts | 6 +++ apps/desktop/desktop_native/napi/index.js | 3 +- apps/desktop/desktop_native/napi/src/lib.rs | 19 +++++++ apps/desktop/src/main/window.main.ts | 35 +++++++++++-- 11 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 apps/desktop/desktop_native/core/src/process_isolation/linux.rs create mode 100644 apps/desktop/desktop_native/core/src/process_isolation/macos.rs create mode 100644 apps/desktop/desktop_native/core/src/process_isolation/mod.rs create mode 100644 apps/desktop/desktop_native/core/src/process_isolation/windows.rs diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index d3bc04b1dc3..16c359cf599 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -496,6 +496,7 @@ dependencies = [ "core-foundation", "gio", "keytar", + "libc", "libsecret", "rand", "retry", diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index b29ea32b60f..8ae4e91c0a2 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -17,6 +17,7 @@ arboard = { version = "=3.4.0", default-features = false, features = [ ] } base64 = "=0.22.1" cbc = { version = "=0.1.2", features = ["alloc"] } +libc = "0.2.155" rand = "=0.8.5" retry = "=2.0.0" scopeguard = "=1.2.0" diff --git a/apps/desktop/desktop_native/core/src/lib.rs b/apps/desktop/desktop_native/core/src/lib.rs index 1b6ee2c771f..d23a285b4ac 100644 --- a/apps/desktop/desktop_native/core/src/lib.rs +++ b/apps/desktop/desktop_native/core/src/lib.rs @@ -3,4 +3,5 @@ pub mod clipboard; pub mod crypto; pub mod error; pub mod password; +pub mod process_isolation; pub mod powermonitor; diff --git a/apps/desktop/desktop_native/core/src/process_isolation/linux.rs b/apps/desktop/desktop_native/core/src/process_isolation/linux.rs new file mode 100644 index 00000000000..ba8734cff7f --- /dev/null +++ b/apps/desktop/desktop_native/core/src/process_isolation/linux.rs @@ -0,0 +1,51 @@ +use anyhow::Result; +use libc::{c_int, self}; +#[cfg(target_env = "gnu")] +use libc::c_uint; + +// RLIMIT_CORE is the maximum size of a core dump file. Setting both to 0 disables core dumps, on crashes +// https://github.com/torvalds/linux/blob/1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0/include/uapi/asm-generic/resource.h#L20 +#[cfg(target_env = "musl")] +const RLIMIT_CORE: c_int = 4; +#[cfg(target_env = "gnu")] +const RLIMIT_CORE: c_uint = 4; + +// PR_SET_DUMPABLE makes it so no other running process (root or same user) can dump the memory of this process +// or attach a debugger to it. +// https://github.com/torvalds/linux/blob/a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6/include/uapi/linux/prctl.h#L14 +const PR_SET_DUMPABLE: c_int = 4; + +pub fn disable_coredumps() -> Result<()> { + let rlimit = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + if unsafe { libc::setrlimit(RLIMIT_CORE, &rlimit) } != 0 { + let e = std::io::Error::last_os_error(); + return Err(anyhow::anyhow!("failed to disable core dumping, memory might be persisted to disk on crashes {}", e)) + } + + Ok(()) +} + +pub fn is_core_dumping_disabled() -> Result { + let mut rlimit = libc::rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + if unsafe { libc::getrlimit(RLIMIT_CORE, &mut rlimit) } != 0 { + let e = std::io::Error::last_os_error(); + return Err(anyhow::anyhow!("failed to get core dump limit {}", e)) + } + + Ok(rlimit.rlim_cur == 0 && rlimit.rlim_max == 0) +} + +pub fn disable_memory_access() -> Result<()> { + if unsafe { libc::prctl(PR_SET_DUMPABLE, 0) } != 0 { + let e = std::io::Error::last_os_error(); + return Err(anyhow::anyhow!("failed to disable memory dumping, memory is dumpable by other processes {}", e)) + } + + Ok(()) +} diff --git a/apps/desktop/desktop_native/core/src/process_isolation/macos.rs b/apps/desktop/desktop_native/core/src/process_isolation/macos.rs new file mode 100644 index 00000000000..04d8f7266c4 --- /dev/null +++ b/apps/desktop/desktop_native/core/src/process_isolation/macos.rs @@ -0,0 +1,13 @@ +use anyhow::{bail, Result}; + +pub fn disable_coredumps() -> Result<()> { + bail!("Not implemented on Mac") +} + +pub fn is_core_dumping_disabled() -> Result { + bail!("Not implemented on Mac") +} + +pub fn disable_memory_access() -> Result<()> { + bail!("Not implemented on Mac") +} diff --git a/apps/desktop/desktop_native/core/src/process_isolation/mod.rs b/apps/desktop/desktop_native/core/src/process_isolation/mod.rs new file mode 100644 index 00000000000..7c9aaf3bcf7 --- /dev/null +++ b/apps/desktop/desktop_native/core/src/process_isolation/mod.rs @@ -0,0 +1,5 @@ +#[cfg_attr(target_os = "linux", path = "linux.rs")] +#[cfg_attr(target_os = "windows", path = "windows.rs")] +#[cfg_attr(target_os = "macos", path = "macos.rs")] +mod process_isolation; +pub use process_isolation::*; diff --git a/apps/desktop/desktop_native/core/src/process_isolation/windows.rs b/apps/desktop/desktop_native/core/src/process_isolation/windows.rs new file mode 100644 index 00000000000..7c7864fbbd7 --- /dev/null +++ b/apps/desktop/desktop_native/core/src/process_isolation/windows.rs @@ -0,0 +1,13 @@ +use anyhow::{bail, Result}; + +pub fn disable_coredumps() -> Result<()> { + bail!("Not implemented on Windows") +} + +pub fn is_core_dumping_disabled() -> Result { + bail!("Not implemented on Windows") +} + +pub fn disable_memory_access() -> Result<()> { + bail!("Not implemented on Windows") +} diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index fdb48543e8d..208bebcb542 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -41,6 +41,12 @@ export namespace clipboards { export function read(): Promise export function write(text: string, password: boolean): Promise } +export namespace processisolations { + export function disableCoredumps(): Promise + export function isCoreDumpingDisabled(): Promise + export function disableMemoryAccess(): Promise +} + export namespace powermonitors { export function onLock(callback: (err: Error | null, ) => any): Promise export function isLockMonitorAvailable(): Promise diff --git a/apps/desktop/desktop_native/napi/index.js b/apps/desktop/desktop_native/napi/index.js index 92e58a21705..680f1302b9a 100644 --- a/apps/desktop/desktop_native/napi/index.js +++ b/apps/desktop/desktop_native/napi/index.js @@ -206,9 +206,10 @@ if (!nativeBinding) { throw new Error(`Failed to load native binding`) } -const { passwords, biometrics, clipboards, powermonitors } = nativeBinding +const { passwords, biometrics, clipboards, processisolations, powermonitors } = nativeBinding module.exports.passwords = passwords module.exports.biometrics = biometrics module.exports.clipboards = clipboards +module.exports.processisolations = processisolations module.exports.powermonitors = powermonitors diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index fdb3efcc095..a4db60d2e2a 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -142,6 +142,25 @@ pub mod clipboards { } } +#[napi] +pub mod processisolations { + #[napi] + pub async fn disable_coredumps() -> napi::Result<()> { + desktop_core::process_isolation::disable_coredumps() + .map_err(|e| napi::Error::from_reason(e.to_string())) + } + #[napi] + pub async fn is_core_dumping_disabled() -> napi::Result { + desktop_core::process_isolation::is_core_dumping_disabled() + .map_err(|e| napi::Error::from_reason(e.to_string())) + } + #[napi] + pub async fn disable_memory_access() -> napi::Result<()> { + desktop_core::process_isolation::disable_memory_access() + .map_err(|e| napi::Error::from_reason(e.to_string())) + } +} + #[napi] pub mod powermonitors { use napi::{threadsafe_function::{ErrorStrategy::CalleeHandled, ThreadsafeFunction, ThreadsafeFunctionCallMode}, tokio}; diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index e690eef71c8..77528994a41 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -8,6 +8,7 @@ import { firstValueFrom } from "rxjs"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { processisolations } from "@bitwarden/desktop-napi"; import { WindowState } from "../platform/models/domain/window-state"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; @@ -31,6 +32,7 @@ export class WindowMain { private windowStateChangeTimer: NodeJS.Timeout; private windowStates: { [key: string]: WindowState } = {}; private enableAlwaysOnTop = false; + private enableRendererProcessForceCrashReload = false; session: Electron.Session; readonly defaultWidth = 950; @@ -53,9 +55,11 @@ export class WindowMain { this.win.setBackgroundColor(await this.getBackgroundColor()); // By default some linux distro collect core dumps on crashes which gets written to disk. - const crashEvent = once(this.win.webContents, "render-process-gone"); - this.win.webContents.forcefullyCrashRenderer(); - await crashEvent; + if (this.enableRendererProcessForceCrashReload) { + const crashEvent = once(this.win.webContents, "render-process-gone"); + this.win.webContents.forcefullyCrashRenderer(); + await crashEvent; + } this.win.webContents.reloadIgnoringCache(); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -101,6 +105,31 @@ export class WindowMain { // initialization and is ready to create browser windows. // Some APIs can only be used after this event occurs. app.on("ready", async () => { + if (isMac() || isWindows()) { + this.enableRendererProcessForceCrashReload = true; + } else if (isLinux() && !isDev()) { + if (await processisolations.isCoreDumpingDisabled()) { + this.logService.info("Coredumps are disabled in renderer process"); + this.enableRendererProcessForceCrashReload = true; + } else { + this.logService.info("Disabling coredumps in main process"); + try { + await processisolations.disableCoredumps(); + } catch (e) { + this.logService.error("Failed to disable coredumps", e); + } + } + + this.logService.info( + "Disabling external memory dumps & debugger access in main process", + ); + try { + await processisolations.disableMemoryAccess(); + } catch (e) { + this.logService.error("Failed to disable memory access", e); + } + } + await this.createWindow(); resolve(); if (this.argvCallback != null) {