From 656f2353692513a9b761341ee6dab7881cfe4e18 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Mon, 25 Apr 2022 17:00:02 +0530 Subject: [PATCH] fix: return boolean from `safeStorage.isEncryptionAvailable()` instead of crashing On Linux, `isEncryptionAvailable()` was crashing instead of returning a boolean before the 'ready' event was emitted by the app. The reason of the crash is that [`CreateKeyStorage()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=74;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0) expects the config to be set but the function responsible for setting the config, [`SetConfig()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=237;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0), is called only after the app is ready inside [`PostCreateMainMessageLoop()`](https://github.com/electron/electron/blob/main/shell/browser/electron_browser_main_parts.cc#L499). So this change patches [`IsEncryptionAvailable()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=245;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0) to return false if the config hasn't been set already. Fixes: https://github.com/electron/electron/issues/32206 Signed-off-by: Darshan Sen --- docs/api/safe-storage.md | 4 +-- patches/chromium/.patches | 1 + ...ryptionavailable_instead_of_crashing.patch | 30 +++++++++++++++++++ spec-main/api-safe-storage-spec.ts | 19 +++++++++++- 4 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 patches/chromium/fix_return_boolean_from_safestorage_isencryptionavailable_instead_of_crashing.patch diff --git a/docs/api/safe-storage.md b/docs/api/safe-storage.md index e96c18bbf0795..471351c9c1cd3 100644 --- a/docs/api/safe-storage.md +++ b/docs/api/safe-storage.md @@ -18,8 +18,8 @@ The `safeStorage` module has the following methods: Returns `boolean` - Whether encryption is available. -On Linux, returns true if the secret key is -available. On MacOS, returns true if Keychain is available. +On Linux, returns true if the app has emitted the `ready` event and the secret key is available. +On MacOS, returns true if Keychain is available. On Windows, returns true once the app has emitted the `ready` event. ### `safeStorage.encryptString(plainText)` diff --git a/patches/chromium/.patches b/patches/chromium/.patches index ff7ade0c3c84f..6274ec3bd779a 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -108,3 +108,4 @@ fix_non-client_mouse_tracking_and_message_bubbling_on_windows.patch build_make_libcxx_abi_unstable_false_for_electron.patch introduce_ozoneplatform_electron_can_call_x11_property.patch make_gtk_getlibgtk_public.patch +fix_return_boolean_from_safestorage_isencryptionavailable_instead_of_crashing.patch diff --git a/patches/chromium/fix_return_boolean_from_safestorage_isencryptionavailable_instead_of_crashing.patch b/patches/chromium/fix_return_boolean_from_safestorage_isencryptionavailable_instead_of_crashing.patch new file mode 100644 index 0000000000000..85336844c7940 --- /dev/null +++ b/patches/chromium/fix_return_boolean_from_safestorage_isencryptionavailable_instead_of_crashing.patch @@ -0,0 +1,30 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Darshan Sen +Date: Mon, 25 Apr 2022 17:03:34 +0530 +Subject: fix: return boolean from `safeStorage.isEncryptionAvailable()` instead of crashing + +On Linux, `isEncryptionAvailable()` was crashing instead of returning a +boolean before the 'ready' event was emitted by the app. The reason of +the crash is that [`CreateKeyStorage()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=74;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0) +expects the config to be set but the function responsible for setting the +config, [`SetConfig()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=237;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0), +is called only after the app is ready inside [`PostCreateMainMessageLoop()`](https://github.com/electron/electron/blob/main/shell/browser/electron_browser_main_parts.cc#L499). +So this change patches [`IsEncryptionAvailable()`](https://source.chromium.org/chromium/chromium/src/+/main:components/os_crypt/os_crypt_linux.cc;l=245;drc=35be6215ec8f09e50176f36753c68f26c63d1885;bpv=1;bpt=0) +to return false if the config hasn't been set already. + +Fixes: https://github.com/electron/electron/issues/32206 +Signed-off-by: Darshan Sen + +diff --git a/os_crypt/os_crypt_linux.cc b/os_crypt/os_crypt_linux.cc +index 07da4e55f1788..2781d060bbe1d 100644 +--- a/os_crypt/os_crypt_linux.cc ++++ b/os_crypt/os_crypt_linux.cc +@@ -242,7 +242,7 @@ void OSCrypt::SetConfig(std::unique_ptr config) { + + // static + bool OSCrypt::IsEncryptionAvailable() { +- return g_get_password[Version::V11](); ++ return g_cache.Get().config && g_get_password[Version::V11](); + } + + // static diff --git a/spec-main/api-safe-storage-spec.ts b/spec-main/api-safe-storage-spec.ts index b89f909513bd1..01e0a6ecd4efd 100644 --- a/spec-main/api-safe-storage-spec.ts +++ b/spec-main/api-safe-storage-spec.ts @@ -1,6 +1,6 @@ import * as cp from 'child_process'; import * as path from 'path'; -import { safeStorage } from 'electron/main'; +import { app, safeStorage } from 'electron/main'; import { expect } from 'chai'; import { emittedOnce } from './events-helpers'; import { ifdescribe } from './spec-helpers'; @@ -12,8 +12,25 @@ import * as fs from 'fs'; * * 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.isEncryptionAvailable() before and after app is ready', () => { + it('isEncryptionAvailable() returns false before app is ready', async () => { + if (!app.isReady()) { + // isEncryptionAvailable() returns false before the app is ready on + // Linux: https://github.com/electron/electron/issues/32206 + // and + // Windows: https://github.com/electron/electron/issues/33640. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform === 'darwin'); + } + await app.whenReady(); + // isEncryptionAvailable() will always return false on CI due to a mocked + // dbus as mentioned above. + expect(safeStorage.isEncryptionAvailable()).to.equal(process.platform !== 'linux'); + }); +}); + ifdescribe(process.platform !== 'linux')('safeStorage module', () => { after(async () => { const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');