Skip to content

Commit

Permalink
feat: expose safestorage backend information on linux (#39155)
Browse files Browse the repository at this point in the history
* feat: expose safestorage backend information on linux

Co-authored-by: deepak1556 <hop2deep@gmail.com>

* Remove gnome-keyring

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

Co-authored-by: deepak1556 <hop2deep@gmail.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: deepak1556 <hop2deep@gmail.com>
  • Loading branch information
trop[bot] and deepak1556 committed Jul 20, 2023
1 parent 77bfa2a commit a99fc5e
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 39 deletions.
25 changes: 25 additions & 0 deletions docs/api/safe-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
40 changes: 29 additions & 11 deletions shell/browser/api/electron_api_safe_storage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand All @@ -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) {
Expand All @@ -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>();
}
Expand Down Expand Up @@ -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 changes: 30 additions & 0 deletions shell/browser/browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 12 additions & 0 deletions shell/browser/browser_process_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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_;
Expand Down
24 changes: 11 additions & 13 deletions shell/browser/electron_browser_main_parts.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions shell/browser/net/system_network_context_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
24 changes: 14 additions & 10 deletions spec/api-safe-storage-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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)) {
Expand All @@ -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';
Expand Down Expand Up @@ -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');
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/api/safe-storage/decrypt-app/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions spec/fixtures/api/safe-storage/encrypt-app/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit a99fc5e

Please sign in to comment.