Browse Source

feat: expose safestorage backend information on linux (#39155)

* feat: expose safestorage backend information on linux

Co-authored-by: deepak1556 <[email protected]>

* Remove gnome-keyring

Refs https://chromium-review.googlesource.com/c/chromium/src/+/4609704

Co-authored-by: deepak1556 <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: deepak1556 <[email protected]>
trop[bot] 1 year ago
parent
commit
a99fc5e40f

+ 25 - 0
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.

+ 29 - 11
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,9 +28,27 @@ 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<BrowserProcessImpl*>(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<BrowserProcessImpl*>(g_browser_process)
+      ->GetLinuxStorageBackend();
 }
+#endif
 
 v8::Local<v8::Value> EncryptString(v8::Isolate* isolate,
                                    const std::string& plaintext) {
@@ -47,8 +59,8 @@ v8::Local<v8::Value> EncryptString(v8::Isolate* isolate,
       return v8::Local<v8::Value>();
     }
     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<v8::Value>();
   }
@@ -128,6 +140,12 @@ void Initialize(v8::Local<v8::Object> 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)

+ 30 - 0
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;
 }

+ 12 - 0
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<PrefService> 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::NetworkQualityTracker> network_quality_tracker_;

+ 11 - 13
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"
@@ -191,18 +192,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)
@@ -577,6 +566,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<base::Environment> 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)

+ 0 - 5
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

+ 14 - 10
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 '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');

+ 3 - 0
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);

+ 3 - 0
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');
   const encryptedString = await writeFile(pathToEncryptedString, encrypted);
   app.quit();