diff --git a/docs/api/safe-storage.md b/docs/api/safe-storage.md index 564419be98446..590c3496c1ae6 100644 --- a/docs/api/safe-storage.md +++ b/docs/api/safe-storage.md @@ -18,8 +18,8 @@ The `safeStorage` module has the following methods: Returns `Boolean` - Whether encryption is available. -On Linux, returns true if the secret key is -available. On MacOS, returns true if Keychain is available. +On Linux, returns true if the app has emitted the `ready` event and the secret key is available. +On MacOS, returns true if Keychain is available. On Windows, returns true once the app has emitted the `ready` event. ### `safeStorage.encryptString(plainText)` diff --git a/shell/browser/api/electron_api_safe_storage.cc b/shell/browser/api/electron_api_safe_storage.cc index ba1396cf28332..7db2bb7718376 100644 --- a/shell/browser/api/electron_api_safe_storage.cc +++ b/shell/browser/api/electron_api_safe_storage.cc @@ -31,12 +31,24 @@ void SetElectronCryptoReady(bool ready) { #endif bool IsEncryptionAvailable() { +#if defined(OS_LINUX) + // Calling IsEncryptionAvailable() before the app is ready results in a crash + // on Linux. + // Refs: https://github.com/electron/electron/issues/32206. + if (!Browser::Get()->is_ready()) + return false; +#endif return OSCrypt::IsEncryptionAvailable(); } v8::Local EncryptString(v8::Isolate* isolate, const std::string& plaintext) { - if (!OSCrypt::IsEncryptionAvailable()) { + if (!IsEncryptionAvailable()) { + if (!Browser::Get()->is_ready()) { + gin_helper::ErrorThrower(isolate).ThrowError( + "safeStorage cannot be used before app is ready"); + return v8::Local(); + } gin_helper::ErrorThrower(isolate).ThrowError( "Error while decrypting the ciphertext provided to " "safeStorage.decryptString. " @@ -59,7 +71,12 @@ v8::Local EncryptString(v8::Isolate* isolate, } std::string DecryptString(v8::Isolate* isolate, v8::Local buffer) { - if (!OSCrypt::IsEncryptionAvailable()) { + if (!IsEncryptionAvailable()) { + if (!Browser::Get()->is_ready()) { + gin_helper::ErrorThrower(isolate).ThrowError( + "safeStorage cannot be used before app is ready"); + return ""; + } gin_helper::ErrorThrower(isolate).ThrowError( "Error while decrypting the ciphertext provided to " "safeStorage.decryptString. " diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 42ad53af25174..eca1573c0a6ba 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -17,6 +17,9 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/icon_manager.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" +#include "components/os_crypt/key_storage_config_linux.h" #include "components/os_crypt/os_crypt.h" #include "content/browser/browser_main_loop.h" // nogncheck #include "content/public/browser/browser_thread.h" @@ -483,6 +486,26 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() { #endif #if defined(OS_LINUX) bluez::DBusBluezManagerWrapperLinux::Initialize(); + + // Set up crypt config. This needs to be done before anything starts the + // network service, as the raw encryption key needs to be shared with the + // network service for encrypted cookie storage. + std::string app_name = electron::Browser::Get()->GetName(); + const base::CommandLine& command_line = + *base::CommandLine::ForCurrentProcess(); + std::unique_ptr config = + std::make_unique(); + // Forward to os_crypt the flag to use a specific password store. + config->store = command_line.GetSwitchValueASCII(::switches::kPasswordStore); + config->product_name = app_name; + config->application_name = app_name; + config->main_thread_runner = base::ThreadTaskRunnerHandle::Get(); + // c.f. + // https://source.chromium.org/chromium/chromium/src/+/master:chrome/common/chrome_switches.cc;l=689;drc=9d82515060b9b75fa941986f5db7390299669ef1 + config->should_use_preference = + command_line.HasSwitch(::switches::kEnableEncryptionSelection); + base::PathService::Get(chrome::DIR_USER_DATA, &config->user_data_path); + OSCrypt::SetConfig(std::move(config)); #endif #if defined(OS_POSIX) // Exit in response to SIGINT, SIGTERM, etc. diff --git a/shell/browser/net/system_network_context_manager.cc b/shell/browser/net/system_network_context_manager.cc index c2162e0ab5ba8..aefdbf01d755d 100644 --- a/shell/browser/net/system_network_context_manager.cc +++ b/shell/browser/net/system_network_context_manager.cc @@ -293,46 +293,14 @@ void SystemNetworkContextManager::OnNetworkServiceCreated( KeychainPassword::GetServiceName() = app_name + " Safe Storage"; KeychainPassword::GetAccountName() = app_name; #endif -#if defined(OS_LINUX) - // c.f. - // https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/net/system_network_context_manager.cc;l=515;drc=9d82515060b9b75fa941986f5db7390299669ef1;bpv=1;bpt=1 - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - - auto config = std::make_unique(); - config->store = command_line.GetSwitchValueASCII(::switches::kPasswordStore); - config->product_name = app_name; - config->application_name = app_name; - config->main_thread_runner = base::ThreadTaskRunnerHandle::Get(); - // c.f. - // https://source.chromium.org/chromium/chromium/src/+/master:chrome/common/chrome_switches.cc;l=689;drc=9d82515060b9b75fa941986f5db7390299669ef1 - config->should_use_preference = - command_line.HasSwitch(::switches::kEnableEncryptionSelection); - base::PathService::Get(chrome::DIR_USER_DATA, &config->user_data_path); -#endif +#if defined(OS_WIN) || defined(OS_MAC) // The OSCrypt keys are process bound, so if network service is out of // process, send it the required key. if (content::IsOutOfProcessNetworkService() && electron::fuses::IsCookieEncryptionEnabled()) { -#if defined(OS_LINUX) - network::mojom::CryptConfigPtr network_crypt_config = - network::mojom::CryptConfig::New(); - network_crypt_config->application_name = config->application_name; - network_crypt_config->product_name = config->product_name; - network_crypt_config->store = config->store; - network_crypt_config->should_use_preference = config->should_use_preference; - network_crypt_config->user_data_path = config->user_data_path; - - network_service->SetCryptConfig(std::move(network_crypt_config)); - -#else network_service->SetEncryptionKey(OSCrypt::GetRawEncryptionKey()); -#endif } - -#if defined(OS_LINUX) - OSCrypt::SetConfig(std::move(config)); #endif #if DCHECK_IS_ON() diff --git a/spec-main/api-safe-storage-spec.ts b/spec-main/api-safe-storage-spec.ts index b89f909513bd1..098095e2fde06 100644 --- a/spec-main/api-safe-storage-spec.ts +++ b/spec-main/api-safe-storage-spec.ts @@ -12,8 +12,27 @@ import * as fs from 'fs'; * * Because all encryption methods are gated by isEncryptionAvailable, the methods will never return the correct values * when run on CI and linux. +* Refs: https://github.com/electron/electron/issues/30424. */ +describe('safeStorage module', () => { + it('safeStorage before and after app is ready', async () => { + const appPath = path.join(__dirname, 'fixtures', 'crash-cases', 'safe-storage'); + const appProcess = cp.spawn(process.execPath, [appPath]); + + let output = ''; + appProcess.stdout.on('data', data => { output += data; }); + appProcess.stderr.on('data', data => { output += data; }); + + const code = (await emittedOnce(appProcess, 'exit'))[0] ?? 1; + + if (code !== 0 && output) { + console.log(output); + } + expect(code).to.equal(0); + }); +}); + ifdescribe(process.platform !== 'linux')('safeStorage module', () => { after(async () => { const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt'); diff --git a/spec-main/fixtures/crash-cases/safe-storage/index.js b/spec-main/fixtures/crash-cases/safe-storage/index.js new file mode 100644 index 0000000000000..151751820a8d5 --- /dev/null +++ b/spec-main/fixtures/crash-cases/safe-storage/index.js @@ -0,0 +1,39 @@ +const { app, safeStorage } = require('electron'); +const { expect } = require('chai'); + +(async () => { + if (!app.isReady()) { + // isEncryptionAvailable() returns false before the app is ready on + // Linux: https://github.com/electron/electron/issues/32206 + // and + // Windows: https://github.com/electron/electron/issues/33640. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform === 'darwin'); + if (safeStorage.isEncryptionAvailable()) { + const plaintext = 'plaintext'; + const ciphertext = safeStorage.encryptString(plaintext); + expect(Buffer.isBuffer(ciphertext)).to.equal(true); + expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext); + } else { + expect(() => safeStorage.encryptString('plaintext')).to.throw(/safeStorage cannot be used before app is ready/); + expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/safeStorage cannot be used before app is ready/); + } + } + await app.whenReady(); + // isEncryptionAvailable() will always return false on CI due to a mocked + // dbus as mentioned above. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform !== 'linux'); + if (safeStorage.isEncryptionAvailable()) { + const plaintext = 'plaintext'; + const ciphertext = safeStorage.encryptString(plaintext); + expect(Buffer.isBuffer(ciphertext)).to.equal(true); + expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext); + } else { + expect(() => safeStorage.encryptString('plaintext')).to.throw(/Encryption is not available/); + expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/Decryption is not available/); + } +})() + .then(app.quit) + .catch((err) => { + console.error(err); + app.exit(1); + });