Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: expose safestorage backend information on linux #39325

Merged
merged 1 commit into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/api/safe-storage.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,31 @@ 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_any` - 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"`. When this value is present the application
will first try to use `libsecret` backend and if it fails will attempt to use `libgnome_keyring`.
* `gnome_libsecret` - When the following command line flag is provided `--password-store="gnome-libsecret"`.
* `gnome_keyring` - When the following command line flag is provided `--password-store="gnome-keyring"`.
* `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 @@ -9,6 +9,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 @@ -19,14 +20,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 @@ -35,9 +29,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 @@ -48,8 +60,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 @@ -129,6 +141,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)
36 changes: 36 additions & 0 deletions shell/browser/browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,42 @@ 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_ANY:
selected_linux_storage_backend_ = "gnome_any";
break;
case os_crypt::SelectedLinuxBackend::GNOME_KEYRING:
selected_linux_storage_backend_ = "gnome_keyring";
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 @@ -22,6 +22,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 @@ -52,6 +56,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 @@ -122,6 +131,9 @@ class BrowserProcessImpl : public BrowserProcess {
std::unique_ptr<device::GeolocationManager> geolocation_manager_;
std::string locale_;
std::string system_locale_;
#if BUILDFLAG(IS_LINUX)
std::string selected_linux_storage_backend_;
#endif

std::unique_ptr<network::NetworkQualityTracker> network_quality_tracker_;
std::unique_ptr<
Expand Down
11 changes: 11 additions & 0 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 @@ -24,6 +25,7 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.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 @@ -566,6 +568,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 @@ -34,7 +34,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 @@ -290,10 +289,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