From df9b1e8205e5ae4902e8bf23e1bc3d1a91b554ae Mon Sep 17 00:00:00 2001 From: varjolintu Date: Tue, 15 Aug 2023 20:17:18 +0300 Subject: [PATCH 1/6] Refactor extension messaging --- keepassxc-browser/background/client.js | 121 +++++++++++++------------ 1 file changed, 65 insertions(+), 56 deletions(-) diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js index 6981384..fc00aa8 100644 --- a/keepassxc-browser/background/client.js +++ b/keepassxc-browser/background/client.js @@ -51,80 +51,85 @@ const kpErrors = { const messageBuffer = { buffer: [], - addMessage(msg) { - if (!this.buffer.includes(msg)) { - this.buffer.push(msg); - } + addMessage(message) { + this.buffer.push(message); }, - matchAndRemove(msg) { - for (let i = 0; i < this.buffer.length; ++i) { - if (msg.nonce && msg.nonce === keepassClient.incrementedNonce(this.buffer[i].nonce)) { - this.buffer.splice(i, 1); - return true; - } + removeMessageFromIndex(index) { + if (this.buffer.length >= index + 1) { + this.buffer.splice(index, 1); } - - return false; } }; +// Basic class for a message to be sent. The Promise inside the class will be resolved when +// the response to the message is received. +class Message { + constructor(request, enableTimeout, timeoutValue) { + this.promise = new Promise((resolve, reject) => { + this.reject = reject; + this.resolve = resolve; + this.enableTimeout = enableTimeout; + + const messageTimeout = timeoutValue || keepassClient.messageTimeout; + + // Handle timeout + if (enableTimeout) { + this.timeout = setTimeout(() => { + const errorMessage = { + action: request.action, + error: kpErrors.getError(kpErrors.TIMEOUT_OR_NOT_CONNECTED), + errorCode: kpErrors.TIMEOUT_OR_NOT_CONNECTED + }; + + keepass.isKeePassXCAvailable = false; + resolve(errorMessage); + }, messageTimeout); + } + }); + } +} + //-------------------------------------------------------------------------- // Messaging //-------------------------------------------------------------------------- -keepassClient.sendNativeMessage = function(request, enableTimeout = false, timeoutValue) { - return new Promise((resolve, reject) => { - let timeout; - const requestAction = request.action; - const ev = keepassClient.nativePort.onMessage; +keepassClient.sendNativeMessage = async function(request, enableTimeout = false, timeoutValue) { + const message = new Message(request, enableTimeout, timeoutValue); + messageBuffer.addMessage({ request: request, message: message }); - const listener = ((port, action) => { - const handler = (msg) => { - if (msg && msg?.action === action) { - // If the request has a separate requestID, check if it matches when there's no nonce (an error message) - const isNotificationOrError = !msg.nonce && (request.requestID === msg.requestID || (msg.error && msg.errorCode)); + // Send the request + if (keepassClient.nativePort) { + keepassClient.nativePort.postMessage(request); + } - // Only resolve a matching response or a notification (without nonce) - if (isNotificationOrError || messageBuffer.matchAndRemove(msg)) { - port.removeListener(handler); - if (enableTimeout) { - clearTimeout(timeout); - } + // Wait for response + return await message.promise; +}; - resolve(msg); - return; - } - } - }; - return handler; - })(ev, requestAction); - ev.addListener(listener); +keepassClient.handleNativeMessage = function(response) { + const isError = Boolean(!response.nonce && response.error && response.errorCode); - const messageTimeout = timeoutValue || keepassClient.messageTimeout; - - // Handle timeouts - if (enableTimeout) { - timeout = setTimeout(() => { - const errorMessage = { - action: requestAction, - error: kpErrors.getError(kpErrors.TIMEOUT_OR_NOT_CONNECTED), - errorCode: kpErrors.TIMEOUT_OR_NOT_CONNECTED - }; - keepass.isKeePassXCAvailable = false; - ev.removeListener(listener.handler); - resolve(errorMessage); - }, messageTimeout); + // Parse through the message buffer to find the corresponding Promise. + for (let i = 0; i < messageBuffer.buffer.length; ++i) { + if (! messageBuffer.buffer[i]) { + continue; } - // Store the request to the buffer - messageBuffer.addMessage(request); + const request = messageBuffer.buffer[i]?.request; + const message = messageBuffer.buffer[i]?.message; + const errorFound = isError && request?.action === response?.action; - // Send the request - if (keepassClient.nativePort) { - keepassClient.nativePort.postMessage(request); + if ((response.nonce && response.nonce === keepassClient.incrementedNonce(request.nonce)) || errorFound) { + if (message.enableTimeout) { + clearTimeout(message.timeout); + } + + message.resolve(response); + messageBuffer.removeMessageFromIndex(i); + return; } - }); + } }; keepassClient.handleResponse = function(response, incrementedNonce, tab) { @@ -341,5 +346,9 @@ keepassClient.onNativeMessage = function(response) { // Handle database lock/unlock status if (response.action === kpActions.DATABASE_LOCKED || response.action === kpActions.DATABASE_UNLOCKED) { keepass.updateDatabase(); + return; } + + // Generic response handling + keepassClient.handleNativeMessage(response); }; From 62c17e38fb144a0dd2e347354b4de8312abbd069 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 17 Aug 2023 16:10:10 +0300 Subject: [PATCH 2/6] Use navigator.locks for messageBuffer --- keepassxc-browser/background/client.js | 40 ++++++++++++++------------ keepassxc-browser/manifest.json | 2 +- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js index fc00aa8..5ed963e 100644 --- a/keepassxc-browser/background/client.js +++ b/keepassxc-browser/background/client.js @@ -96,7 +96,9 @@ class Message { keepassClient.sendNativeMessage = async function(request, enableTimeout = false, timeoutValue) { const message = new Message(request, enableTimeout, timeoutValue); - messageBuffer.addMessage({ request: request, message: message }); + await navigator.locks.request('messageBuffer', async (lock) => { + messageBuffer.addMessage({ request: request, message: message }); + }); // Send the request if (keepassClient.nativePort) { @@ -107,29 +109,31 @@ keepassClient.sendNativeMessage = async function(request, enableTimeout = false, return await message.promise; }; -keepassClient.handleNativeMessage = function(response) { +keepassClient.handleNativeMessage = async function(response) { const isError = Boolean(!response.nonce && response.error && response.errorCode); // Parse through the message buffer to find the corresponding Promise. - for (let i = 0; i < messageBuffer.buffer.length; ++i) { - if (! messageBuffer.buffer[i]) { - continue; - } - - const request = messageBuffer.buffer[i]?.request; - const message = messageBuffer.buffer[i]?.message; - const errorFound = isError && request?.action === response?.action; - - if ((response.nonce && response.nonce === keepassClient.incrementedNonce(request.nonce)) || errorFound) { - if (message.enableTimeout) { - clearTimeout(message.timeout); + await navigator.locks.request('messageBuffer', async (lock) => { + for (let i = 0; i < messageBuffer.buffer.length; ++i) { + if (! messageBuffer.buffer[i]) { + continue; } - message.resolve(response); - messageBuffer.removeMessageFromIndex(i); - return; + const request = messageBuffer.buffer[i]?.request; + const message = messageBuffer.buffer[i]?.message; + const errorFound = isError && request?.action === response?.action; + + if ((response.nonce && response.nonce === keepassClient.incrementedNonce(request.nonce)) || errorFound) { + if (message.enableTimeout) { + clearTimeout(message.timeout); + } + + message.resolve(response); + messageBuffer.removeMessageFromIndex(i); + return; + } } - } + }); }; keepassClient.handleResponse = function(response, incrementedNonce, tab) { diff --git a/keepassxc-browser/manifest.json b/keepassxc-browser/manifest.json index 117e22e..b7410f9 100755 --- a/keepassxc-browser/manifest.json +++ b/keepassxc-browser/manifest.json @@ -159,7 +159,7 @@ "applications": { "gecko": { "id": "keepassxc-browser@keepassxc.org", - "strict_min_version": "93.0" + "strict_min_version": "96.0" } }, "default_locale": "en" From eea5b78f9892789d56b786d8bed7d722f6eefae2 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Sun, 20 Aug 2023 10:26:37 +0300 Subject: [PATCH 3/6] Prevent multiple password generator requests --- keepassxc-browser/background/keepass.js | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index 7a02908..ab65fac 100755 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -15,6 +15,7 @@ keepass.latestVersionUrl = 'https://api.github.com/repos/keepassxreboot/keepassx keepass.cacheTimeout = 30 * 1000; // Milliseconds keepass.databaseHash = ''; keepass.previousDatabaseHash = ''; +keepass.passwordRequestSent = false; keepass.reconnectLoop = null; const kpActions = { @@ -168,18 +169,24 @@ keepass.retrieveCredentials = async function(tab, args = []) { keepass.generatePassword = async function(tab) { if (!keepass.isConnected) { - return []; + return ''; + } + + if (keepass.passwordRequestSent) { + return undefined; + } else { + keepass.passwordRequestSent = true; } try { const taResponse = await keepass.testAssociation(tab); if (!taResponse) { browserAction.showDefault(tab); - return []; + return ''; } if (!keepass.compareVersion(keepass.requiredKeePassXC, keepass.currentKeePassXC)) { - return []; + return ''; } let password; @@ -201,10 +208,12 @@ keepass.generatePassword = async function(tab) { logError('generatePassword rejected'); } + keepass.passwordRequestSent = false; return password; } catch (err) { logError(`generatePassword failed: ${err}`); - return []; + keepass.passwordRequestSent = false; + return undefined; } }; From e6dfb5bd160a40d58cf454e6c7898cde1c17f706 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Mon, 21 Aug 2023 08:44:19 +0300 Subject: [PATCH 4/6] Cleanup --- keepassxc-browser/background/client.js | 15 +++++++-------- keepassxc-browser/background/keepass.js | 9 --------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js index 5ed963e..624a994 100644 --- a/keepassxc-browser/background/client.js +++ b/keepassxc-browser/background/client.js @@ -56,7 +56,7 @@ const messageBuffer = { }, removeMessageFromIndex(index) { - if (this.buffer.length >= index + 1) { + if (index >= 0 && index < this.buffer.length) { this.buffer.splice(index, 1); } } @@ -95,17 +95,16 @@ class Message { //-------------------------------------------------------------------------- keepassClient.sendNativeMessage = async function(request, enableTimeout = false, timeoutValue) { + if (!keepassClient.nativePort) { + return; + } + const message = new Message(request, enableTimeout, timeoutValue); await navigator.locks.request('messageBuffer', async (lock) => { messageBuffer.addMessage({ request: request, message: message }); }); - // Send the request - if (keepassClient.nativePort) { - keepassClient.nativePort.postMessage(request); - } - - // Wait for response + keepassClient.nativePort.postMessage(request); return await message.promise; }; @@ -115,7 +114,7 @@ keepassClient.handleNativeMessage = async function(response) { // Parse through the message buffer to find the corresponding Promise. await navigator.locks.request('messageBuffer', async (lock) => { for (let i = 0; i < messageBuffer.buffer.length; ++i) { - if (! messageBuffer.buffer[i]) { + if (!messageBuffer.buffer[i]) { continue; } diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index ab65fac..3a749f0 100755 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -15,7 +15,6 @@ keepass.latestVersionUrl = 'https://api.github.com/repos/keepassxreboot/keepassx keepass.cacheTimeout = 30 * 1000; // Milliseconds keepass.databaseHash = ''; keepass.previousDatabaseHash = ''; -keepass.passwordRequestSent = false; keepass.reconnectLoop = null; const kpActions = { @@ -169,13 +168,7 @@ keepass.retrieveCredentials = async function(tab, args = []) { keepass.generatePassword = async function(tab) { if (!keepass.isConnected) { - return ''; - } - - if (keepass.passwordRequestSent) { return undefined; - } else { - keepass.passwordRequestSent = true; } try { @@ -208,11 +201,9 @@ keepass.generatePassword = async function(tab) { logError('generatePassword rejected'); } - keepass.passwordRequestSent = false; return password; } catch (err) { logError(`generatePassword failed: ${err}`); - keepass.passwordRequestSent = false; return undefined; } }; From 196ec78763c12dba6fbb3e9e7e22a52c1be2d5b1 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Sun, 10 Sep 2023 16:45:54 +0300 Subject: [PATCH 5/6] Add error messages --- keepassxc-browser/background/client.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js index 624a994..559ebd7 100644 --- a/keepassxc-browser/background/client.js +++ b/keepassxc-browser/background/client.js @@ -96,6 +96,7 @@ class Message { keepassClient.sendNativeMessage = async function(request, enableTimeout = false, timeoutValue) { if (!keepassClient.nativePort) { + logError('No native messaging port defined.'); return; } @@ -132,6 +133,8 @@ keepassClient.handleNativeMessage = async function(response) { return; } } + + logError('Corresponding request not found in the message buffer for response: ', response); }); }; From 559ac56f50715560faa1b3d15e10649f612496e5 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Sun, 10 Sep 2023 17:08:53 +0300 Subject: [PATCH 6/6] Switch to debugLogMessage --- keepassxc-browser/background/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js index 559ebd7..0e926df 100644 --- a/keepassxc-browser/background/client.js +++ b/keepassxc-browser/background/client.js @@ -134,7 +134,7 @@ keepassClient.handleNativeMessage = async function(response) { } } - logError('Corresponding request not found in the message buffer for response: ', response); + debugLogMessage('Corresponding request not found in the message buffer for response: ', response); }); };