From 9946f612963a2ab5be97749f9a76a2d8ca3ac8e5 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Thu, 4 Sep 2025 12:40:37 -0400 Subject: [PATCH 1/4] fix(notifications): [PM-25424] Fix unnecessary quick reconnect * Ensure we don't reconnect on feature flag emissions of the same value * Harden notification processing * Do error for both --- .../default-server-notifications.service.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts index e8ac93dc61f..6d0728ab65d 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts @@ -172,6 +172,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private hasAccessToken$(userId: UserId) { return this.configService.getFeatureFlag$(FeatureFlag.PushNotificationsWhenLocked).pipe( + distinctUntilChanged(), switchMap((featureFlagEnabled) => { if (featureFlagEnabled) { return this.authService.authStatusFor$(userId).pipe( @@ -305,11 +306,23 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer startListening() { return this.notifications$ .pipe( - mergeMap(async ([notification, userId]) => this.processNotification(notification, userId)), + mergeMap(async ([notification, userId]) => { + try { + await this.processNotification(notification, userId); + } catch (err: unknown) { + this.logService.error( + `Problem processing notification of type ${notification.type}`, + err, + ); + } + }), ) .subscribe({ - error: (e: unknown) => - this.logService.warning("Error in server notifications$ observable", e), + error: (err: unknown) => + this.logService.error( + "Fatal error in server notifications$ observable, notifications won't be recieved anymore.", + err, + ), }); } From bff18a8cd24d54f1859953a6d433f42a1fe66179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Thu, 4 Sep 2025 19:37:40 +0200 Subject: [PATCH 2/4] [PM-25131] Initialize provider keys on the SDK (#16183) * [PM-25131] Initialize provider keys on the SDK * Remove null default * Typechecking --- .../services/sdk/default-sdk.service.ts | 9 +-- .../src/abstractions/key.service.ts | 5 +- libs/key-management/src/key.service.ts | 55 +++++++++++++++++-- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.ts b/libs/common/src/platform/services/sdk/default-sdk.service.ts index 210e1bdc59b..2713aaf8f4b 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.ts @@ -27,10 +27,9 @@ import { UnsignedSharedKey, } from "@bitwarden/sdk-internal"; -import { EncryptedOrganizationKeyData } from "../../../admin-console/models/data/encrypted-organization-key.data"; import { AccountInfo, AccountService } from "../../../auth/abstractions/account.service"; import { DeviceType } from "../../../enums/device-type.enum"; -import { EncryptedString } from "../../../key-management/crypto/models/enc-string"; +import { EncryptedString, EncString } from "../../../key-management/crypto/models/enc-string"; import { OrganizationId, UserId } from "../../../types/guid"; import { UserKey } from "../../../types/key"; import { Environment, EnvironmentService } from "../../abstractions/environment.service"; @@ -220,7 +219,7 @@ export class DefaultSdkService implements SdkService { kdfParams: KdfConfig, privateKey: EncryptedString, userKey: UserKey, - orgKeys: Record | null, + orgKeys: Record, ) { await client.crypto().initialize_user_crypto({ userId: asUuid(userId), @@ -245,9 +244,7 @@ export class DefaultSdkService implements SdkService { // null to make sure any existing org keys are cleared. await client.crypto().initialize_org_crypto({ organizationKeys: new Map( - Object.entries(orgKeys ?? {}) - .filter(([_, v]) => v.type === "organization") - .map(([k, v]) => [asUuid(k), v.key as UnsignedSharedKey]), + Object.entries(orgKeys).map(([k, v]) => [asUuid(k), v.toJSON() as UnsignedSharedKey]), ), }); diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index 1685938de3d..0f9618cbab9 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -1,6 +1,5 @@ import { Observable } from "rxjs"; -import { EncryptedOrganizationKeyData } from "@bitwarden/common/admin-console/models/data/encrypted-organization-key.data"; import { ProfileOrganizationResponse } from "@bitwarden/common/admin-console/models/response/profile-organization.response"; import { ProfileProviderOrganizationResponse } from "@bitwarden/common/admin-console/models/response/profile-provider-organization.response"; import { ProfileProviderResponse } from "@bitwarden/common/admin-console/models/response/profile-provider.response"; @@ -406,9 +405,7 @@ export abstract class KeyService { * @deprecated Temporary function to allow the SDK to be initialized after the login process, it * will be removed when auth has been migrated to the SDK. */ - abstract encryptedOrgKeys$( - userId: UserId, - ): Observable | null>; + abstract encryptedOrgKeys$(userId: UserId): Observable>; /** * Gets an observable stream of the users public key. If the user is does not have diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index ed0b844a2a4..53f7c6ed158 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -907,10 +907,57 @@ export class DefaultKeyService implements KeyServiceAbstraction { return this.cipherDecryptionKeys$(userId).pipe(map((keys) => keys?.orgKeys ?? null)); } - encryptedOrgKeys$( - userId: UserId, - ): Observable | null> { - return this.stateProvider.getUser(userId, USER_ENCRYPTED_ORGANIZATION_KEYS).state$; + encryptedOrgKeys$(userId: UserId): Observable> { + return this.userPrivateKey$(userId)?.pipe( + switchMap((userPrivateKey) => { + if (userPrivateKey == null) { + // We can't do any org based decryption + return of({}); + } + + return combineLatest([ + this.stateProvider.getUser(userId, USER_ENCRYPTED_ORGANIZATION_KEYS).state$, + this.providerKeysHelper$(userId, userPrivateKey), + ]).pipe( + switchMap(async ([encryptedOrgKeys, providerKeys]) => { + const userPubKey = await this.derivePublicKey(userPrivateKey); + + const result: Record = {}; + encryptedOrgKeys = encryptedOrgKeys ?? {}; + for (const orgId of Object.keys(encryptedOrgKeys) as OrganizationId[]) { + if (result[orgId] != null) { + continue; + } + const encrypted = BaseEncryptedOrganizationKey.fromData(encryptedOrgKeys[orgId]); + if (encrypted == null) { + continue; + } + + let orgKey: EncString; + + // Because the SDK only supports user encrypted org keys, we need to re-encrypt + // any provider encrypted org keys with the user's public key. This should be removed + // once the SDK has support for provider keys. + if (BaseEncryptedOrganizationKey.isProviderEncrypted(encrypted)) { + if (providerKeys == null) { + continue; + } + orgKey = await this.encryptService.encapsulateKeyUnsigned( + await encrypted.decrypt(this.encryptService, providerKeys!), + userPubKey!, + ); + } else { + orgKey = encrypted.encryptedOrganizationKey; + } + + result[orgId] = orgKey; + } + + return result; + }), + ); + }), + ); } cipherDecryptionKeys$(userId: UserId): Observable { From ca9b531571df519f22a3b04412c2c70f47089ae4 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Thu, 4 Sep 2025 14:31:52 -0500 Subject: [PATCH 3/4] avoid using the SDK to decrypt attachments for emergency access (#16293) - The SDK does not have emergency access functionality built in at this point. --- libs/common/src/vault/abstractions/cipher.service.ts | 5 +++++ libs/common/src/vault/services/cipher.service.ts | 4 +++- .../download-attachment/download-attachment.component.ts | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 7eb2d4b0656..7971b6d4658 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -257,6 +257,10 @@ export abstract class CipherService implements UserKeyRotationDataProvider; /** diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index f6e12e71edd..2ad4274c235 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1544,11 +1544,13 @@ export class CipherService implements CipherServiceAbstraction { return encryptedCiphers; } + /** @inheritdoc */ async getDecryptedAttachmentBuffer( cipherId: CipherId, attachment: AttachmentView, response: Response, userId: UserId, + useLegacyDecryption?: boolean, ): Promise { const useSdkDecryption = await this.configService.getFeatureFlag( FeatureFlag.PM19941MigrateCipherDomainToSdk, @@ -1558,7 +1560,7 @@ export class CipherService implements CipherServiceAbstraction { this.ciphers$(userId).pipe(map((ciphersData) => new Cipher(ciphersData[cipherId]))), ); - if (useSdkDecryption) { + if (useSdkDecryption && !useLegacyDecryption) { const encryptedContent = await response.arrayBuffer(); return this.cipherEncryptionService.decryptAttachmentContent( cipherDomain, diff --git a/libs/vault/src/components/download-attachment/download-attachment.component.ts b/libs/vault/src/components/download-attachment/download-attachment.component.ts index 627ffafc6b2..8208887b888 100644 --- a/libs/vault/src/components/download-attachment/download-attachment.component.ts +++ b/libs/vault/src/components/download-attachment/download-attachment.component.ts @@ -92,6 +92,9 @@ export class DownloadAttachmentComponent { this.attachment, response, userId, + // When the emergency access ID is present, the cipher is being viewed via emergency access. + // Force legacy decryption in these cases. + this.emergencyAccessId ? true : false, ); this.fileDownloadService.download({ From ea1c3252e8d1d5ae2c82fd5059c5a5e2894c28e9 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 4 Sep 2025 21:40:25 +0200 Subject: [PATCH 4/4] [BEEEP | PM-25358] Add process isolation on windows and mac desktop main process (#16156) * Prevent memory dumping and debugger on windows and mac main process * Fix clippy * Only isolate process when isdev is false * Clean up * Add backticks around link --- apps/desktop/desktop_native/Cargo.lock | 26 +++++++++++++ apps/desktop/desktop_native/Cargo.toml | 1 + apps/desktop/desktop_native/core/Cargo.toml | 1 + .../core/src/process_isolation/linux.rs | 12 +++++- .../core/src/process_isolation/macos.rs | 15 ++++++- .../core/src/process_isolation/mod.rs | 13 +++++++ .../core/src/process_isolation/windows.rs | 15 ++++++- apps/desktop/desktop_native/napi/index.d.ts | 2 +- apps/desktop/desktop_native/napi/src/lib.rs | 4 +- apps/desktop/src/main/window.main.ts | 39 ++++++++++--------- 10 files changed, 101 insertions(+), 27 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index bb7f7d9995b..23deda915ed 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -922,6 +922,7 @@ dependencies = [ "rsa", "russh-cryptovec", "scopeguard", + "secmem-proc", "security-framework", "security-framework-sys", "sha2", @@ -2769,6 +2770,16 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "rustix-linux-procfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" +dependencies = [ + "once_cell", + "rustix 1.0.7", +] + [[package]] name = "rustversion" version = "1.0.20" @@ -2847,6 +2858,21 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secmem-proc" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "473559b1d28f530c3a9b5f91a2866053e2b1c528a0e43dae83048139c99490c2" +dependencies = [ + "anyhow", + "cfg-if", + "libc", + "rustix 1.0.7", + "rustix-linux-procfs", + "thiserror 2.0.12", + "windows 0.61.1", +] + [[package]] name = "security-framework" version = "3.1.0" diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index 84b835de35f..7e5ab43c6b9 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -48,6 +48,7 @@ rand = "=0.9.1" rsa = "=0.9.6" russh-cryptovec = "=0.7.3" scopeguard = "=1.2.0" +secmem-proc = "=0.3.7" security-framework = "=3.1.0" security-framework-sys = "=2.13.0" serde = "=1.0.209" diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 97189e1d7cd..3aedf90613e 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -38,6 +38,7 @@ rand = { workspace = true } rsa = { workspace = true } russh-cryptovec = { workspace = true } scopeguard = { workspace = true } +secmem-proc = { workspace = true } sha2 = { workspace = true } ssh-encoding = { workspace = true } ssh-key = { workspace = true, features = [ diff --git a/apps/desktop/desktop_native/core/src/process_isolation/linux.rs b/apps/desktop/desktop_native/core/src/process_isolation/linux.rs index dc027e0b546..395d722ea01 100644 --- a/apps/desktop/desktop_native/core/src/process_isolation/linux.rs +++ b/apps/desktop/desktop_native/core/src/process_isolation/linux.rs @@ -20,6 +20,8 @@ pub fn disable_coredumps() -> Result<()> { rlim_cur: 0, rlim_max: 0, }; + println!("[Process Isolation] Disabling core dumps via setrlimit"); + if unsafe { libc::setrlimit(RLIMIT_CORE, &rlimit) } != 0 { let e = std::io::Error::last_os_error(); return Err(anyhow::anyhow!( @@ -44,11 +46,17 @@ pub fn is_core_dumping_disabled() -> Result { Ok(rlimit.rlim_cur == 0 && rlimit.rlim_max == 0) } -pub fn disable_memory_access() -> Result<()> { +pub fn isolate_process() -> Result<()> { + let pid = std::process::id(); + println!( + "[Process Isolation] Disabling ptrace and memory access for main ({}) via PR_SET_DUMPABLE", + pid + ); + 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 {}", + "failed to disable memory dumping, memory may be accessible by other processes {}", e )); } diff --git a/apps/desktop/desktop_native/core/src/process_isolation/macos.rs b/apps/desktop/desktop_native/core/src/process_isolation/macos.rs index 04d8f7266c4..ce42e06a832 100644 --- a/apps/desktop/desktop_native/core/src/process_isolation/macos.rs +++ b/apps/desktop/desktop_native/core/src/process_isolation/macos.rs @@ -8,6 +8,17 @@ pub fn is_core_dumping_disabled() -> Result { bail!("Not implemented on Mac") } -pub fn disable_memory_access() -> Result<()> { - bail!("Not implemented on Mac") +pub fn isolate_process() -> Result<()> { + let pid: u32 = std::process::id(); + println!( + "[Process Isolation] Disabling ptrace on main process ({}) via PT_DENY_ATTACH", + pid + ); + + secmem_proc::harden_process().map_err(|e| { + anyhow::anyhow!( + "failed to disable memory dumping, memory may be accessible by other processes {}", + e + ) + }) } diff --git a/apps/desktop/desktop_native/core/src/process_isolation/mod.rs b/apps/desktop/desktop_native/core/src/process_isolation/mod.rs index 30f4dbf689a..b1872c8a423 100644 --- a/apps/desktop/desktop_native/core/src/process_isolation/mod.rs +++ b/apps/desktop/desktop_native/core/src/process_isolation/mod.rs @@ -1,3 +1,16 @@ +//! This module implements process isolation, which aims to protect +//! a process from dumping memory to disk when crashing, and from +//! userspace memory access. +//! +//! On Windows, by default most userspace apps can read the memory of all +//! other apps, and attach debuggers. On Mac, this is not possible, and only +//! after granting developer permissions can an app attach to processes via +//! ptrace / read memory. On Linux, this depends on the distro / configuration of yama +//! `https://linux-audit.com/protect-ptrace-processes-kernel-yama-ptrace_scope/` +//! For instance, ubuntu prevents ptrace of other processes by default. +//! On Fedora, there are change proposals but ptracing is still possible unless +//! otherwise configured. + #[allow(clippy::module_inception)] #[cfg_attr(target_os = "linux", path = "linux.rs")] #[cfg_attr(target_os = "windows", path = "windows.rs")] diff --git a/apps/desktop/desktop_native/core/src/process_isolation/windows.rs b/apps/desktop/desktop_native/core/src/process_isolation/windows.rs index 7c7864fbbd7..dc1092f9131 100644 --- a/apps/desktop/desktop_native/core/src/process_isolation/windows.rs +++ b/apps/desktop/desktop_native/core/src/process_isolation/windows.rs @@ -8,6 +8,17 @@ pub fn is_core_dumping_disabled() -> Result { bail!("Not implemented on Windows") } -pub fn disable_memory_access() -> Result<()> { - bail!("Not implemented on Windows") +pub fn isolate_process() -> Result<()> { + let pid: u32 = std::process::id(); + println!( + "[Process Isolation] Isolating main process via DACL {}", + pid + ); + + secmem_proc::harden_process().map_err(|e| { + anyhow::anyhow!( + "failed to isolate process, memory may be accessible by other processes {}", + e + ) + }) } diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index a920f0c00aa..281bfd5d69f 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -82,7 +82,7 @@ export declare namespace sshagent { export declare namespace processisolations { export function disableCoredumps(): Promise export function isCoreDumpingDisabled(): Promise - export function disableMemoryAccess(): Promise + export function isolateProcess(): Promise } export declare namespace powermonitors { export function onLock(callback: (err: Error | null, ) => any): Promise diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 1f99c1c3ed2..0e5cdc838d7 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -337,8 +337,8 @@ pub mod processisolations { #[allow(clippy::unused_async)] // FIXME: Remove unused async! #[napi] - pub async fn disable_memory_access() -> napi::Result<()> { - desktop_core::process_isolation::disable_memory_access() + pub async fn isolate_process() -> napi::Result<()> { + desktop_core::process_isolation::isolate_process() .map_err(|e| napi::Error::from_reason(e.to_string())) } } diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index 5b81cf8140b..1595252251b 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -36,7 +36,7 @@ export class WindowMain { private windowStateChangeTimer: NodeJS.Timeout; private windowStates: { [key: string]: WindowState } = {}; private enableAlwaysOnTop = false; - private enableRendererProcessForceCrashReload = false; + private enableRendererProcessForceCrashReload = true; session: Electron.Session; readonly defaultWidth = 950; @@ -149,28 +149,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"); + if (!isDev()) { + // This currently breaks the file portal for snap https://github.com/flatpak/xdg-desktop-portal/issues/785 + if (!isSnapStore()) { + this.logService.info( + "[Process Isolation] Isolating process from debuggers and memory dumps", + ); try { - await processisolations.disableCoredumps(); + await processisolations.isolateProcess(); } catch (e) { - this.logService.error("Failed to disable coredumps", e); + this.logService.error("[Process Isolation] Failed to isolate main process", e); } } - // this currently breaks the file portal for snap https://github.com/flatpak/xdg-desktop-portal/issues/785 - if (!isSnapStore()) { - this.logService.info("Disabling memory dumps in main process"); - try { - await processisolations.disableMemoryAccess(); - } catch (e) { - this.logService.error("Failed to disable memory dumps", e); + if (isLinux()) { + if (await processisolations.isCoreDumpingDisabled()) { + this.logService.info("Coredumps are disabled in renderer process"); + } else { + this.enableRendererProcessForceCrashReload = false; + this.logService.info("Disabling coredumps in main process"); + try { + await processisolations.disableCoredumps(); + this.enableRendererProcessForceCrashReload = true; + } catch (e) { + this.logService.error("Failed to disable coredumps", e); + } } } }