From 4273a263a6c3bc79507a333b067e444cd48c87f8 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Thu, 22 Jan 2026 13:42:45 +0000 Subject: [PATCH] session key retrieval redesign for the local backed session storage --- .../browser/src/background/main.background.ts | 51 +++----- .../services/browser-local-storage.service.ts | 16 +++ .../local-backed-session-storage.service.ts | 113 +++++++++++++++--- 3 files changed, 129 insertions(+), 51 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9d551ec2622..afc1c747c13 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -139,8 +139,6 @@ import { IpcService, IpcSessionRepository } from "@bitwarden/common/platform/ipc import { Message, MessageListener, MessageSender } from "@bitwarden/common/platform/messaging"; // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; -import { Lazy } from "@bitwarden/common/platform/misc/lazy"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { ServerNotificationsService } from "@bitwarden/common/platform/server-notifications"; // eslint-disable-next-line no-restricted-imports -- Needed for service creation import { @@ -325,7 +323,10 @@ import BrowserLocalStorageService from "../platform/services/browser-local-stora import BrowserMemoryStorageService from "../platform/services/browser-memory-storage.service"; import { BrowserScriptInjectorService } from "../platform/services/browser-script-injector.service"; import I18nService from "../platform/services/i18n.service"; -import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service"; +import { + LocalBackedSessionStorageService, + SessionKeyResolveService, +} from "../platform/services/local-backed-session-storage.service"; import { BackgroundPlatformUtilsService } from "../platform/services/platform-utils/background-platform-utils.service"; import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service"; import { PopupRouterCacheBackgroundService } from "../platform/services/popup-router-cache-background.service"; @@ -562,36 +563,22 @@ export default class MainBackground { this.memoryStorageService = this.memoryStorageForStateProviders; } - if (BrowserApi.isManifestVersion(3)) { - // Creates a session key for mv3 storage of large memory items - const sessionKey = new Lazy(async () => { - // Key already in session storage - const sessionStorage = new BrowserMemoryStorageService(); - const existingKey = await sessionStorage.get("session-key"); - if (existingKey) { - if (sessionStorage.valuesRequireDeserialization) { - return SymmetricCryptoKey.fromJSON(existingKey); - } - return existingKey; - } + this.encryptService = new EncryptServiceImplementation( + this.cryptoFunctionService, + this.logService, + true, + ); - // New key - const { derivedKey } = await this.keyGenerationService.createKeyWithPurpose( - 128, - "ephemeral", - "bitwarden-ephemeral", - ); - await sessionStorage.save("session-key", derivedKey.toJSON()); - return derivedKey; - }); + if (BrowserApi.isManifestVersion(3)) { + const sessionKeyResolveService = new SessionKeyResolveService( + new BrowserMemoryStorageService(), + this.keyGenerationService, + ); this.largeObjectMemoryStorageForStateProviders = new LocalBackedSessionStorageService( - sessionKey, + sessionKeyResolveService, this.storageService, - // For local backed session storage, we expect that the encrypted data on disk will persist longer than the encryption key in memory - // and failures to decrypt because of that are completely expected. For this reason, we pass in `false` to the `EncryptServiceImplementation` - // so that MAC failures are not logged. - new EncryptServiceImplementation(this.cryptoFunctionService, this.logService, false), + this.encryptService, this.platformUtilsService, this.logService, ); @@ -626,12 +613,6 @@ export default class MainBackground { storageServiceProvider, ); - this.encryptService = new EncryptServiceImplementation( - this.cryptoFunctionService, - this.logService, - true, - ); - this.singleUserStateProvider = new DefaultSingleUserStateProvider( storageServiceProvider, stateEventRegistrarService, diff --git a/apps/browser/src/platform/services/browser-local-storage.service.ts b/apps/browser/src/platform/services/browser-local-storage.service.ts index 30454cf6a77..e34c3ac4904 100644 --- a/apps/browser/src/platform/services/browser-local-storage.service.ts +++ b/apps/browser/src/platform/services/browser-local-storage.service.ts @@ -15,6 +15,22 @@ export default class BrowserLocalStorageService extends AbstractChromeStorageSer return await this.getWithRetries(key, 0); } + /** + * Retrieves all storage keys. + * + * Returns all keys stored in local storage when the browser supports the getKeys API (Chrome 130+). + * Returns an empty array on older browser versions where this feature is unavailable. + * + * @returns Array of storage keys, or empty array if the feature is not supported + */ + async getKeys(): Promise { + // getKeys function is only available since Chrome 130 + if ("getKeys" in this.chromeStorageApi) { + return this.chromeStorageApi.getKeys(); + } + return []; + } + private async getWithRetries(key: string, retryNum: number): Promise { // See: https://github.com/EFForg/privacybadger/pull/2980 const MAX_RETRIES = 5; diff --git a/apps/browser/src/platform/services/local-backed-session-storage.service.ts b/apps/browser/src/platform/services/local-backed-session-storage.service.ts index d0613ee644c..4367230119a 100644 --- a/apps/browser/src/platform/services/local-backed-session-storage.service.ts +++ b/apps/browser/src/platform/services/local-backed-session-storage.service.ts @@ -2,6 +2,7 @@ // @ts-strict-ignore import { Subject } from "rxjs"; +import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -12,27 +13,78 @@ import { StorageUpdate, } from "@bitwarden/common/platform/abstractions/storage.service"; import { compareValues } from "@bitwarden/common/platform/misc/compare-values"; -import { Lazy } from "@bitwarden/common/platform/misc/lazy"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { StorageService } from "@bitwarden/storage-core"; import { BrowserApi } from "../browser/browser-api"; import { MemoryStoragePortMessage } from "../storage/port-messages"; import { portName } from "../storage/port-name"; +import BrowserLocalStorageService from "./browser-local-storage.service"; + +const SESSION_KEY_PREFIX = "session_"; + +/** + * Manages an ephemeral session key for encrypting session storage items persisted in local storage. + * + * The session key is stored in session storage and automatically cleared when the browser session ends + * (e.g., browser restart, extension reload). When the session key is unavailable, any encrypted items + * in local storage cannot be decrypted and must be cleared to maintain data consistency. + * + * This provides session-scoped security for sensitive data while using persistent local storage as the backing store. + */ +export class SessionKeyResolveService { + constructor( + private readonly storageService: StorageService, + private readonly keyGenerationService: KeyGenerationService, + ) {} + + /** + * Retrieves the session key from storage. + * + * @return session key or null when not in storage + */ + async get(): Promise { + const key = await this.storageService.get("session-key"); + if (key) { + if (this.storageService.valuesRequireDeserialization) { + return SymmetricCryptoKey.fromJSON(key); + } + return key; + } + return null; + } + + /** + * Creates new session key and adds it to underlying storage. + * + * @return newly created session key + */ + async create(): Promise { + const { derivedKey } = await this.keyGenerationService.createKeyWithPurpose( + 128, + "ephemeral", + "bitwarden-ephemeral", + ); + await sessionStorage.save("session-key", derivedKey.toJSON()); + return derivedKey; + } +} + export class LocalBackedSessionStorageService extends AbstractStorageService implements ObservableStorageService { + readonly valuesRequireDeserialization = true; private ports: Set = new Set([]); private cache: Record = {}; private updatesSubject = new Subject(); - readonly valuesRequireDeserialization = true; updates$ = this.updatesSubject.asObservable(); constructor( - private readonly sessionKey: Lazy>, - private readonly localStorage: AbstractStorageService, + private readonly sessionKeyResolveService: SessionKeyResolveService, + private readonly localStorage: BrowserLocalStorageService, private readonly encryptService: EncryptService, private readonly platformUtilsService: PlatformUtilsService, private readonly logService: LogService, @@ -74,7 +126,7 @@ export class LocalBackedSessionStorageService return this.cache[key] as T; } - const value = await this.getLocalSessionValue(await this.sessionKey.get(), key); + const value = await this.getLocalSessionValue(await this.getSessionKey(), key); if (this.cache[key] === undefined && value !== undefined) { // Cache is still empty and we just got a value from local/session storage, cache it. @@ -136,6 +188,44 @@ export class LocalBackedSessionStorageService this.updatesSubject.next({ key, updateType: "remove" }); } + protected broadcastMessage(data: Omit) { + this.ports.forEach((port) => { + this.sendMessageTo(port, data); + }); + } + + private async getSessionKey(): Promise { + const sessionKey = await this.sessionKeyResolveService.get(); + if (sessionKey != null) { + return sessionKey; + } + + // Session key is missing (browser restart/extension reload), so all stored session data + // cannot be decrypted. Clear all items before creating a new session key. + await this.clear(); + + return await this.sessionKeyResolveService.create(); + } + + /** + * Removes all stored session data. + * + * Called when the session key is unavailable (typically after browser restart or extension reload), + * making all encrypted session data unrecoverable. Prevents orphaned encrypted data from accumulating. + */ + private async clear() { + const keys = (await this.localStorage.getKeys()).filter((key) => + key.startsWith(SESSION_KEY_PREFIX), + ); + this.logService.debug( + `[LocalBackedSessionStorageService] Clearing local session storage. Found ${keys}`, + ); + for (const key of keys) { + const keyWithoutPrefix = key.substring(SESSION_KEY_PREFIX.indexOf(key) + 1); + await this.remove(keyWithoutPrefix); + } + } + private async getLocalSessionValue(encKey: SymmetricCryptoKey, key: string): Promise { const local = await this.localStorage.get(this.sessionStorageKey(key)); if (local == null) { @@ -159,10 +249,7 @@ export class LocalBackedSessionStorageService } const valueJson = JSON.stringify(value); - const encValue = await this.encryptService.encryptString( - valueJson, - await this.sessionKey.get(), - ); + const encValue = await this.encryptService.encryptString(valueJson, await this.getSessionKey()); await this.localStorage.save(this.sessionStorageKey(key), encValue.encryptedString); } @@ -197,12 +284,6 @@ export class LocalBackedSessionStorageService }); } - protected broadcastMessage(data: Omit) { - this.ports.forEach((port) => { - this.sendMessageTo(port, data); - }); - } - private sendMessageTo( port: chrome.runtime.Port, data: Omit, @@ -214,7 +295,7 @@ export class LocalBackedSessionStorageService } private sessionStorageKey(key: string) { - return `session_${key}`; + return `${SESSION_KEY_PREFIX}${key}`; } private compareValues(value1: T, value2: T): boolean {