Skip to content

Commit

Permalink
fix: return boolean from safeStorage.isEncryptionAvailable() instea…
Browse files Browse the repository at this point in the history
…d 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 changes `IsEncryptionAvailable()` to return `false` when the app
is not ready on Linux.

Fixes: #32206
Signed-off-by: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
RaisinTen committed Apr 26, 2022
1 parent f3e0517 commit 73832a7
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
4 changes: 2 additions & 2 deletions docs/api/safe-storage.md
Expand Up @@ -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)`
Expand Down
7 changes: 7 additions & 0 deletions shell/browser/api/electron_api_safe_storage.cc
Expand Up @@ -31,6 +31,13 @@ void SetElectronCryptoReady(bool ready) {
#endif

bool IsEncryptionAvailable() {
#if BUILDFLAG(IS_LINUX)
// Calling IsEncryptionAvailable() before the app is ready results in a crash
// on Linux.
// Refs: https://github.com/electron/electron/issues/32206.
if (!Browser::Get()->is_ready())
return false;
#endif
return OSCrypt::IsEncryptionAvailable();
}

Expand Down
19 changes: 18 additions & 1 deletion 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';
Expand All @@ -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');
Expand Down

0 comments on commit 73832a7

Please sign in to comment.