From cc408d0a72c05d359f683f59553dfe8181b84be2 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 31 Aug 2026 11:55:29 -0400 Subject: [PATCH] Report the real cause of an OS keystore failure Every failure shows "User canceled OS unlock entry" no matter what went wrong. Test the store for the actual state, and give callers a message describing what couldn't be accessed. (cherry picked from commit ace9fa0227023f9a32e77cb35b76feaaf81bc7b9) --- chrome/content/zotero/xpcom/osKeyStore.js | 42 +++++++++++++++++++++-- chrome/locale/en-US/zotero/zotero.ftl | 7 ++++ test/tests/webdavTest.js | 17 +++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/osKeyStore.js b/chrome/content/zotero/xpcom/osKeyStore.js index 5d4c9d1d05..c46d8fb5cf 100644 --- a/chrome/content/zotero/xpcom/osKeyStore.js +++ b/chrome/content/zotero/xpcom/osKeyStore.js @@ -101,6 +101,32 @@ Zotero.OSKeyStore = { ); }, + // Mozilla's OSKeyStore reports every failure as a canceled unlock prompt, whatever the + // cause, so probe the native store to record what actually went wrong and return an error + // with a message that can be shown to the user + _error: async function (e, stringName) { + let detail; + // Make sure the label from Mozilla's module still exists + let label = this._module && this._module.STORE_LABEL; + if (typeof label != 'string' || !label) { + detail = "store label unavailable"; + } + else { + try { + let keyStore = Cc["@mozilla.org/security/oskeystore;1"] + .getService(Ci.nsIOSKeyStore); + detail = (await keyStore.asyncSecretAvailable(label)) + ? "secret is available" + : "no secret stored"; + } + catch (probeError) { + detail = "key store unusable: " + probeError; + } + } + Zotero.debug(`OS key store failure (${detail}): ${e}`, 1); + return new Zotero.Error(Zotero.getString(stringName), 0, { keyStoreError: e }); + }, + // Returns prefixed ciphertext. Throws if OSKeyStore is unavailable so we // don't silently store plaintext when a caller expects encryption. encrypt: async function (plaintext) { @@ -108,7 +134,13 @@ Zotero.OSKeyStore = { if (!mod) { throw new Error("OSKeyStore unavailable"); } - let ciphertext = await mod.encrypt(plaintext); + let ciphertext; + try { + ciphertext = await mod.encrypt(plaintext); + } + catch (e) { + throw await this._error(e, 'os-keystore-save-failed'); + } return this._prefix + ciphertext; }, @@ -127,7 +159,13 @@ Zotero.OSKeyStore = { // OSKeyStore.encrypt() encodes the string as UTF-8 before encrypting, but // OSKeyStore.decrypt() returns the decrypted bytes as a binary string, so // decode it here - let binaryStr = await mod.decrypt(value.slice(this._prefix.length)); + let binaryStr; + try { + binaryStr = await mod.decrypt(value.slice(this._prefix.length)); + } + catch (e) { + throw await this._error(e, 'os-keystore-read-failed'); + } return new TextDecoder().decode( Uint8Array.from(binaryStr, char => char.charCodeAt(0)) ); diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index 2e56030328..897360fdf0 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -1142,6 +1142,13 @@ os-keystore-save-failed = *[other] { -app-name } couldn’t access your { -os-name } keyring to securely save your credentials. Make sure a keyring service such as GNOME Keyring or KWallet is running and try again. } +os-keystore-read-failed = + { PLATFORM() -> + [macos] { -app-name } couldn’t access the { -os-name } Keychain to read your saved credentials. Make sure your Keychain is accessible and try again. + [windows] { -app-name } couldn’t use { -os-name } Credential Manager to read your saved credentials. Try again or restart { -app-name }. + *[other] { -app-name } couldn’t access your { -os-name } keyring to read your saved credentials. Make sure a keyring service such as GNOME Keyring or KWallet is running and try again. + } + os-keystore-save-unencrypted = { -app-name } can save your credentials unencrypted instead. Anyone with access to your { -app-name } profile folder would then be able to read them. os-keystore-save-unencrypted-button = Save Anyway diff --git a/test/tests/webdavTest.js b/test/tests/webdavTest.js index b1f5bd601f..564ae405f8 100644 --- a/test/tests/webdavTest.js +++ b/test/tests/webdavTest.js @@ -261,6 +261,23 @@ describe("Zotero.Sync.Storage.Mode.WebDAV", function () { }) + it("should report a keystore read failure with a usable message", async function () { + if (!Zotero.OSKeyStore.available) { + this.skip(); + } + await controller.setPassword("password"); + var stub = sinon.stub(Zotero.OSKeyStore._module, "decrypt") + .rejects(new Error("User canceled OS unlock entry")); + try { + let e = await getPromiseError(controller.getPassword()); + assert.equal(e.message, Zotero.getString('os-keystore-read-failed')); + } + finally { + stub.restore(); + } + }) + + it("shouldn't store a password that would read back as encrypted", async function () { var encryptStub = sinon.stub(Zotero.OSKeyStore, "encrypt") .rejects(new Error("User canceled OS unlock entry"));