From 34e7c3696a1375080197ebc68c222d221fbc2ef3 Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 13 Jul 2023 18:14:33 +0900 Subject: [PATCH] feat: expose safestorage backend information on linux (#38873) * feat: expose safestorage backend information on linux * Remove gnome-keyring Refs https://chromium-review.googlesource.com/c/chromium/src/+/4609704 --- docs/api/safe-storage.md | 25 ++++++++++++ .../browser/api/electron_api_safe_storage.cc | 40 ++++++++++++++----- shell/browser/browser_process_impl.cc | 30 ++++++++++++++ shell/browser/browser_process_impl.h | 12 ++++++ shell/browser/electron_browser_main_parts.cc | 24 +++++------ .../net/system_network_context_manager.cc | 5 --- spec/api-safe-storage-spec.ts | 24 ++++++----- .../api/safe-storage/decrypt-app/main.js | 3 ++ .../api/safe-storage/encrypt-app/main.js | 3 ++ 9 files changed, 127 insertions(+), 39 deletions(-) diff --git a/docs/api/safe-storage.md b/docs/api/safe-storage.md index 471351c9c1c..c8d370c9b32 100644 --- a/docs/api/safe-storage.md +++ b/docs/api/safe-storage.md @@ -38,3 +38,28 @@ Returns `string` - the decrypted string. Decrypts the encrypted buffer obtained with `safeStorage.encryptString` back into a string. This function will throw an error if decryption fails. + +### `safeStorage.setUsePlainTextEncryption(usePlainText)` + +* `usePlainText` boolean + +This function on Linux will force the module to use an in memory password for creating +symmetric key that is used for encrypt/decrypt functions when a valid OS password +manager cannot be determined for the current active desktop environment. This function +is a no-op on Windows and MacOS. + +### `safeStorage.getSelectedStorageBackend()` _Linux_ + +Returns `string` - User friendly name of the password manager selected on Linux. + +This function will return one of the following values: + +* `basic_text` - When the desktop environment is not recognised or if the following +command line flag is provided `--password-store="basic"`. +* `gnome_libsecret` - When the desktop environment is `X-Cinnamon`, `Deepin`, `GNOME`, `Pantheon`, `XFCE`, `UKUI`, `unity` or if the following command line flag is provided `--password-store="gnome-libsecret"`. +* `kwallet` - When the desktop session is `kde4` or if the following command line flag +is provided `--password-store="kwallet"`. +* `kwallet5` - When the desktop session is `kde5` or if the following command line flag +is provided `--password-store="kwallet5"`. +* `kwallet6` - When the desktop session is `kde6`. +* `unknown` - When the function is called before app has emitted the `ready` event. diff --git a/shell/browser/api/electron_api_safe_storage.cc b/shell/browser/api/electron_api_safe_storage.cc index 7bdc1504399..1eaddeb1124 100644 --- a/shell/browser/api/electron_api_safe_storage.cc +++ b/shell/browser/api/electron_api_safe_storage.cc @@ -8,6 +8,7 @@ #include "components/os_crypt/sync/os_crypt.h" #include "shell/browser/browser.h" +#include "shell/browser/browser_process_impl.h" #include "shell/common/gin_converters/base_converter.h" #include "shell/common/gin_converters/callback_converter.h" #include "shell/common/gin_helper/dictionary.h" @@ -18,14 +19,7 @@ namespace electron::safestorage { static const char* kEncryptionVersionPrefixV10 = "v10"; static const char* kEncryptionVersionPrefixV11 = "v11"; - -#if DCHECK_IS_ON() -static bool electron_crypto_ready = false; - -void SetElectronCryptoReady(bool ready) { - electron_crypto_ready = ready; -} -#endif +static bool use_password_v10 = false; bool IsEncryptionAvailable() { #if BUILDFLAG(IS_LINUX) @@ -34,10 +28,28 @@ bool IsEncryptionAvailable() { // Refs: https://github.com/electron/electron/issues/32206. if (!Browser::Get()->is_ready()) return false; -#endif + return OSCrypt::IsEncryptionAvailable() || + (use_password_v10 && + static_cast(g_browser_process) + ->GetLinuxStorageBackend() == "basic_text"); +#else return OSCrypt::IsEncryptionAvailable(); +#endif } +void SetUsePasswordV10(bool use) { + use_password_v10 = use; +} + +#if BUILDFLAG(IS_LINUX) +std::string GetSelectedLinuxBackend() { + if (!Browser::Get()->is_ready()) + return "unknown"; + return static_cast(g_browser_process) + ->GetLinuxStorageBackend(); +} +#endif + v8::Local EncryptString(v8::Isolate* isolate, const std::string& plaintext) { if (!IsEncryptionAvailable()) { @@ -47,8 +59,8 @@ v8::Local EncryptString(v8::Isolate* isolate, return v8::Local(); } gin_helper::ErrorThrower(isolate).ThrowError( - "Error while decrypting the ciphertext provided to " - "safeStorage.decryptString. " + "Error while encrypting the text provided to " + "safeStorage.encryptString. " "Encryption is not available."); return v8::Local(); } @@ -128,6 +140,12 @@ void Initialize(v8::Local exports, &electron::safestorage::IsEncryptionAvailable); dict.SetMethod("encryptString", &electron::safestorage::EncryptString); dict.SetMethod("decryptString", &electron::safestorage::DecryptString); + dict.SetMethod("setUsePlainTextEncryption", + &electron::safestorage::SetUsePasswordV10); +#if BUILDFLAG(IS_LINUX) + dict.SetMethod("getSelectedStorageBackend", + &electron::safestorage::GetSelectedLinuxBackend); +#endif } NODE_LINKED_BINDING_CONTEXT_AWARE(electron_browser_safe_storage, Initialize) diff --git a/shell/browser/browser_process_impl.cc b/shell/browser/browser_process_impl.cc index 6f960959926..ad2f6ee294a 100644 --- a/shell/browser/browser_process_impl.cc +++ b/shell/browser/browser_process_impl.cc @@ -305,6 +305,36 @@ const std::string& BrowserProcessImpl::GetSystemLocale() const { return system_locale_; } +#if BUILDFLAG(IS_LINUX) +void BrowserProcessImpl::SetLinuxStorageBackend( + os_crypt::SelectedLinuxBackend selected_backend) { + switch (selected_backend) { + case os_crypt::SelectedLinuxBackend::BASIC_TEXT: + selected_linux_storage_backend_ = "basic_text"; + break; + case os_crypt::SelectedLinuxBackend::GNOME_LIBSECRET: + selected_linux_storage_backend_ = "gnome_libsecret"; + break; + case os_crypt::SelectedLinuxBackend::KWALLET: + selected_linux_storage_backend_ = "kwallet"; + break; + case os_crypt::SelectedLinuxBackend::KWALLET5: + selected_linux_storage_backend_ = "kwallet5"; + break; + case os_crypt::SelectedLinuxBackend::KWALLET6: + selected_linux_storage_backend_ = "kwallet6"; + break; + case os_crypt::SelectedLinuxBackend::DEFER: + NOTREACHED(); + break; + } +} + +const std::string& BrowserProcessImpl::GetLinuxStorageBackend() const { + return selected_linux_storage_backend_; +} +#endif // BUILDFLAG(IS_LINUX) + void BrowserProcessImpl::SetApplicationLocale(const std::string& locale) { locale_ = locale; } diff --git a/shell/browser/browser_process_impl.h b/shell/browser/browser_process_impl.h index 44ebba43420..0e8e2c79d73 100644 --- a/shell/browser/browser_process_impl.h +++ b/shell/browser/browser_process_impl.h @@ -23,6 +23,10 @@ #include "services/network/public/cpp/shared_url_loader_factory.h" #include "shell/browser/net/system_network_context_manager.h" +#if BUILDFLAG(IS_LINUX) +#include "components/os_crypt/sync/key_storage_util_linux.h" +#endif + namespace printing { class PrintJobManager; } @@ -53,6 +57,11 @@ class BrowserProcessImpl : public BrowserProcess { void SetSystemLocale(const std::string& locale); const std::string& GetSystemLocale() const; +#if BUILDFLAG(IS_LINUX) + void SetLinuxStorageBackend(os_crypt::SelectedLinuxBackend selected_backend); + const std::string& GetLinuxStorageBackend() const; +#endif + void EndSession() override {} void FlushLocalStateAndReply(base::OnceClosure reply) override {} bool IsShuttingDown() override; @@ -120,6 +129,9 @@ class BrowserProcessImpl : public BrowserProcess { std::unique_ptr local_state_; std::string locale_; std::string system_locale_; +#if BUILDFLAG(IS_LINUX) + std::string selected_linux_storage_backend_; +#endif embedder_support::OriginTrialsSettingsStorage origin_trials_settings_storage_; std::unique_ptr network_quality_tracker_; diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 8795668266a..b11b7c02ce0 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -14,6 +14,7 @@ #include "base/feature_list.h" #include "base/i18n/rtl.h" #include "base/metrics/field_trial.h" +#include "base/nix/xdg_util.h" #include "base/path_service.h" #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" @@ -23,8 +24,8 @@ #include "chrome/browser/ui/color/chrome_color_mixers.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" -#include "components/embedder_support/origin_trials/origin_trials_settings_storage.h" #include "components/os_crypt/sync/key_storage_config_linux.h" +#include "components/os_crypt/sync/key_storage_util_linux.h" #include "components/os_crypt/sync/os_crypt.h" #include "content/browser/browser_main_loop.h" // nogncheck #include "content/public/browser/browser_child_process_host_delegate.h" @@ -192,18 +193,6 @@ void UpdateDarkThemeSetting() { } #endif -// A fake BrowserProcess object that used to feed the source code from chrome. -class FakeBrowserProcessImpl : public BrowserProcessImpl { - public: - embedder_support::OriginTrialsSettingsStorage* - GetOriginTrialsSettingsStorage() override { - return &origin_trials_settings_storage_; - } - - private: - embedder_support::OriginTrialsSettingsStorage origin_trials_settings_storage_; -}; - } // namespace #if BUILDFLAG(IS_LINUX) @@ -578,6 +567,15 @@ void ElectronBrowserMainParts::PostCreateMainMessageLoop() { config->should_use_preference = command_line.HasSwitch(::switches::kEnableEncryptionSelection); base::PathService::Get(DIR_SESSION_DATA, &config->user_data_path); + + bool use_backend = !config->should_use_preference || + os_crypt::GetBackendUse(config->user_data_path); + std::unique_ptr env(base::Environment::Create()); + base::nix::DesktopEnvironment desktop_env = + base::nix::GetDesktopEnvironment(env.get()); + os_crypt::SelectedLinuxBackend selected_backend = + os_crypt::SelectBackend(config->store, use_backend, desktop_env); + fake_browser_process_->SetLinuxStorageBackend(selected_backend); OSCrypt::SetConfig(std::move(config)); #endif #if BUILDFLAG(IS_MAC) diff --git a/shell/browser/net/system_network_context_manager.cc b/shell/browser/net/system_network_context_manager.cc index b548d38a998..209c368cb5f 100644 --- a/shell/browser/net/system_network_context_manager.cc +++ b/shell/browser/net/system_network_context_manager.cc @@ -35,7 +35,6 @@ #include "services/network/public/cpp/features.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/mojom/network_context.mojom.h" -#include "shell/browser/api/electron_api_safe_storage.h" #include "shell/browser/browser.h" #include "shell/browser/electron_browser_client.h" #include "shell/common/application_info.h" @@ -291,10 +290,6 @@ void SystemNetworkContextManager::OnNetworkServiceCreated( electron::fuses::IsCookieEncryptionEnabled()) { network_service->SetEncryptionKey(OSCrypt::GetRawEncryptionKey()); } - -#if DCHECK_IS_ON() - electron::safestorage::SetElectronCryptoReady(true); -#endif } network::mojom::NetworkContextParamsPtr diff --git a/spec/api-safe-storage-spec.ts b/spec/api-safe-storage-spec.ts index 1f48175017e..740e916f130 100644 --- a/spec/api-safe-storage-spec.ts +++ b/spec/api-safe-storage-spec.ts @@ -6,15 +6,6 @@ import { ifdescribe } from './lib/spec-helpers'; import * as fs from 'fs-extra'; import { once } from 'node:events'; -/* isEncryptionAvailable returns false in Linux when running CI due to a mocked dbus. This stops -* Chrome from reaching the system's keyring or libsecret. When running the tests with config.store -* set to basic-text, a nullptr is returned from chromium, defaulting the available encryption to false. -* -* 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'); @@ -33,7 +24,13 @@ describe('safeStorage module', () => { }); }); -ifdescribe(process.platform !== 'linux')('safeStorage module', () => { +describe('safeStorage module', () => { + before(() => { + if (process.platform === 'linux') { + safeStorage.setUsePlainTextEncryption(true); + } + }); + after(async () => { const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt'); if (await fs.pathExists(pathToEncryptedString)) { @@ -47,6 +44,12 @@ ifdescribe(process.platform !== 'linux')('safeStorage module', () => { }); }); + ifdescribe(process.platform === 'linux')('SafeStorage.getSelectedStorageBackend()', () => { + it('should return a valid backend', () => { + expect(safeStorage.getSelectedStorageBackend()).to.equal('basic_text'); + }); + }); + describe('SafeStorage.encryptString()', () => { it('valid input should correctly encrypt string', () => { const plaintext = 'plaintext'; @@ -87,6 +90,7 @@ ifdescribe(process.platform !== 'linux')('safeStorage module', () => { }).to.throw(Error); }); }); + describe('safeStorage persists encryption key across app relaunch', () => { it('can decrypt after closing and reopening app', async () => { const fixturesPath = path.resolve(__dirname, 'fixtures'); diff --git a/spec/fixtures/api/safe-storage/decrypt-app/main.js b/spec/fixtures/api/safe-storage/decrypt-app/main.js index 72eee87044a..3476034455d 100644 --- a/spec/fixtures/api/safe-storage/decrypt-app/main.js +++ b/spec/fixtures/api/safe-storage/decrypt-app/main.js @@ -6,6 +6,9 @@ const pathToEncryptedString = path.resolve(__dirname, '..', 'encrypted.txt'); const readFile = fs.readFile; app.whenReady().then(async () => { + if (process.platform === 'linux') { + safeStorage.setUsePlainTextEncryption(true); + } const encryptedString = await readFile(pathToEncryptedString); const decrypted = safeStorage.decryptString(encryptedString); console.log(decrypted); diff --git a/spec/fixtures/api/safe-storage/encrypt-app/main.js b/spec/fixtures/api/safe-storage/encrypt-app/main.js index ae4a11bff89..a9f6d261c02 100644 --- a/spec/fixtures/api/safe-storage/encrypt-app/main.js +++ b/spec/fixtures/api/safe-storage/encrypt-app/main.js @@ -6,6 +6,9 @@ const pathToEncryptedString = path.resolve(__dirname, '..', 'encrypted.txt'); const writeFile = fs.writeFile; app.whenReady().then(async () => { + if (process.platform === 'linux') { + safeStorage.setUsePlainTextEncryption(true); + } const encrypted = safeStorage.encryptString('plaintext'); await writeFile(pathToEncryptedString, encrypted); app.quit();