Browse Source

chore: revert 459fe0e from 26-x-y (#39119)

Revert "feat: expose safestorage backend information on linux (#39107)"

This reverts commit 459fe0e68d2c1961e68b071ebc93ea9456457454.
Robo 1 year ago
parent
commit
25c18637ed

+ 0 - 25
docs/api/safe-storage.md

@@ -38,28 +38,3 @@ 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.

+ 11 - 29
shell/browser/api/electron_api_safe_storage.cc

@@ -8,7 +8,6 @@
 
 #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"
@@ -19,7 +18,14 @@ namespace electron::safestorage {
 
 static const char* kEncryptionVersionPrefixV10 = "v10";
 static const char* kEncryptionVersionPrefixV11 = "v11";
-static bool use_password_v10 = false;
+
+#if DCHECK_IS_ON()
+static bool electron_crypto_ready = false;
+
+void SetElectronCryptoReady(bool ready) {
+  electron_crypto_ready = ready;
+}
+#endif
 
 bool IsEncryptionAvailable() {
 #if BUILDFLAG(IS_LINUX)
@@ -28,28 +34,10 @@ bool IsEncryptionAvailable() {
   // Refs: https://github.com/electron/electron/issues/32206.
   if (!Browser::Get()->is_ready())
     return false;
-  return OSCrypt::IsEncryptionAvailable() ||
-         (use_password_v10 &&
-          static_cast<BrowserProcessImpl*>(g_browser_process)
-                  ->GetLinuxStorageBackend() == "basic_text");
-#else
-  return OSCrypt::IsEncryptionAvailable();
 #endif
+  return OSCrypt::IsEncryptionAvailable();
 }
 
-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) {
   if (!IsEncryptionAvailable()) {
@@ -59,8 +47,8 @@ v8::Local<v8::Value> EncryptString(v8::Isolate* isolate,
       return v8::Local<v8::Value>();
     }
     gin_helper::ErrorThrower(isolate).ThrowError(
-        "Error while encrypting the text provided to "
-        "safeStorage.encryptString. "
+        "Error while decrypting the ciphertext provided to "
+        "safeStorage.decryptString. "
         "Encryption is not available.");
     return v8::Local<v8::Value>();
   }
@@ -140,12 +128,6 @@ 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)

+ 0 - 30
shell/browser/browser_process_impl.cc

@@ -305,36 +305,6 @@ 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;
 }

+ 0 - 12
shell/browser/browser_process_impl.h

@@ -23,10 +23,6 @@
 #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;
 }
@@ -57,11 +53,6 @@ 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;
@@ -129,9 +120,6 @@ 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_;

+ 13 - 11
shell/browser/electron_browser_main_parts.cc

@@ -14,7 +14,6 @@
 #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"
@@ -24,8 +23,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,6 +191,18 @@ 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)
@@ -566,15 +577,6 @@ 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)

+ 5 - 0
shell/browser/net/system_network_context_manager.cc

@@ -35,6 +35,7 @@
 #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"
@@ -290,6 +291,10 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
       electron::fuses::IsCookieEncryptionEnabled()) {
     network_service->SetEncryptionKey(OSCrypt::GetRawEncryptionKey());
   }
+
+#if DCHECK_IS_ON()
+  electron::safestorage::SetElectronCryptoReady(true);
+#endif
 }
 
 network::mojom::NetworkContextParamsPtr

+ 10 - 14
spec/api-safe-storage-spec.ts

@@ -6,6 +6,15 @@ 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');
@@ -24,13 +33,7 @@ describe('safeStorage module', () => {
   });
 });
 
-describe('safeStorage module', () => {
-  before(() => {
-    if (process.platform === 'linux') {
-      safeStorage.setUsePlainTextEncryption(true);
-    }
-  });
-
+ifdescribe(process.platform !== 'linux')('safeStorage module', () => {
   after(async () => {
     const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');
     if (await fs.pathExists(pathToEncryptedString)) {
@@ -44,12 +47,6 @@ describe('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';
@@ -90,7 +87,6 @@ describe('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');

+ 0 - 3
spec/fixtures/api/safe-storage/decrypt-app/main.js

@@ -6,9 +6,6 @@ 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);

+ 0 - 3
spec/fixtures/api/safe-storage/encrypt-app/main.js

@@ -6,9 +6,6 @@ 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();