fix: fix a crash in `safeStorage` on Linux (#34147)

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 and uses that instead of the raw API in other
places like `EncryptString()` and `DecryptString()`.

Fixes: https://github.com/electron/electron/issues/32206
Signed-off-by: Darshan Sen <[email protected]>

Co-authored-by: Darshan Sen <[email protected]>

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)`

shell/browser/api/electron_api_safe_storage.cc

@@ -31,12 +31,24 @@ 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();
 }
 
 v8::Local<v8::Value> EncryptString(v8::Isolate* isolate,
                                    const std::string& plaintext) {
-  if (!OSCrypt::IsEncryptionAvailable()) {
+  if (!IsEncryptionAvailable()) {
+    if (!Browser::Get()->is_ready()) {
+      gin_helper::ErrorThrower(isolate).ThrowError(
+          "safeStorage cannot be used before app is ready");
+      return v8::Local<v8::Value>();
+    }
     gin_helper::ErrorThrower(isolate).ThrowError(
         "Error while decrypting the ciphertext provided to "
         "safeStorage.decryptString. "
@@ -59,7 +71,12 @@ v8::Local<v8::Value> EncryptString(v8::Isolate* isolate,
 }
 
 std::string DecryptString(v8::Isolate* isolate, v8::Local<v8::Value> buffer) {
-  if (!OSCrypt::IsEncryptionAvailable()) {
+  if (!IsEncryptionAvailable()) {
+    if (!Browser::Get()->is_ready()) {
+      gin_helper::ErrorThrower(isolate).ThrowError(
+          "safeStorage cannot be used before app is ready");
+      return "";
+    }
     gin_helper::ErrorThrower(isolate).ThrowError(
         "Error while decrypting the ciphertext provided to "
         "safeStorage.decryptString. "

spec-main/api-safe-storage-spec.ts

@@ -12,8 +12,27 @@ 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 module', () => {
+  it('safeStorage before and after app is ready', async () => {
+    const appPath = path.join(__dirname, 'fixtures', 'crash-cases', 'safe-storage');
+    const appProcess = cp.spawn(process.execPath, [appPath]);
+
+    let output = '';
+    appProcess.stdout.on('data', data => { output += data; });
+    appProcess.stderr.on('data', data => { output += data; });
+
+    const code = (await emittedOnce(appProcess, 'exit'))[0] ?? 1;
+
+    if (code !== 0 && output) {
+      console.log(output);
+    }
+    expect(code).to.equal(0);
+  });
+});
+
 ifdescribe(process.platform !== 'linux')('safeStorage module', () => {
   after(async () => {
     const pathToEncryptedString = path.resolve(__dirname, 'fixtures', 'api', 'safe-storage', 'encrypted.txt');

spec-main/fixtures/crash-cases/safe-storage/index.js

@@ -0,0 +1,39 @@
+const { app, safeStorage } = require('electron');
+const { expect } = require('chai');
+
+(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');
+    if (safeStorage.isEncryptionAvailable()) {
+      const plaintext = 'plaintext';
+      const ciphertext = safeStorage.encryptString(plaintext);
+      expect(Buffer.isBuffer(ciphertext)).to.equal(true);
+      expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext);
+    } else {
+      expect(() => safeStorage.encryptString('plaintext')).to.throw(/safeStorage cannot be used before app is ready/);
+      expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/safeStorage cannot be used before app is ready/);
+    }
+  }
+  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');
+  if (safeStorage.isEncryptionAvailable()) {
+    const plaintext = 'plaintext';
+    const ciphertext = safeStorage.encryptString(plaintext);
+    expect(Buffer.isBuffer(ciphertext)).to.equal(true);
+    expect(safeStorage.decryptString(ciphertext)).to.equal(plaintext);
+  } else {
+    expect(() => safeStorage.encryptString('plaintext')).to.throw(/Encryption is not available/);
+    expect(() => safeStorage.decryptString(Buffer.from(''))).to.throw(/Decryption is not available/);
+  }
+})()
+  .then(app.quit)
+  .catch((err) => {
+    console.error(err);
+    app.exit(1);
+  });