From 441416cd0943c16337500d7df5b96d9f3cd7e1e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sami=20V=C3=A4nttinen?= <sami.vanttinen@protonmail.com>
Date: Sun, 20 Feb 2022 22:05:10 +0200
Subject: [PATCH] Handle requestID with error responses (#1561)

Handle requestID with error responses
---
 keepassxc-browser/background/client.js  | 16 ++++++++++++++--
 keepassxc-browser/background/keepass.js |  3 ++-
 keepassxc-protocol.md                   | 19 ++++++++-----------
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/keepassxc-browser/background/client.js b/keepassxc-browser/background/client.js
index a251b94..9553702 100644
--- a/keepassxc-browser/background/client.js
+++ b/keepassxc-browser/background/client.js
@@ -82,8 +82,11 @@ keepassClient.sendNativeMessage = function(request, enableTimeout = false, timeo
         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;
+
                     // Only resolve a matching response or a notification (without nonce)
-                    if (!msg.nonce || messageBuffer.matchAndRemove(msg)) {
+                    if (isNotificationOrError || messageBuffer.matchAndRemove(msg)) {
                         port.removeListener(handler);
                         if (enableTimeout) {
                             clearTimeout(timeout);
@@ -162,9 +165,13 @@ keepassClient.buildRequest = function(action, encrypted, nonce, clientID, trigge
 
 keepassClient.sendMessage = async function(kpAction, tab, messageData, nonce, enableTimeout = false, triggerUnlock = false) {
     const request = keepassClient.buildRequest(kpAction, keepassClient.encrypt(messageData, nonce), nonce, keepass.clientID, triggerUnlock);
-    const response = await keepassClient.sendNativeMessage(request, enableTimeout);
+    if (messageData.requestID) {
+        request["requestID"] = messageData.requestID;
+    }
 
+    const response = await keepassClient.sendNativeMessage(request, enableTimeout);
     const incrementedNonce = keepassClient.incrementedNonce(nonce);
+
     return keepassClient.handleResponse(response, incrementedNonce, tab);
 };
 
@@ -176,6 +183,11 @@ keepassClient.getNonce = function() {
     return nacl.util.encodeBase64(nacl.randomBytes(keepassClient.keySize));
 };
 
+// Creates a random 8 character string for Request ID
+keepassClient.getRequestId = function() {
+    return Math.random().toString(16).substring(2, 10);
+};
+
 keepassClient.incrementedNonce = function(nonce) {
     const oldNonce = nacl.util.decodeBase64(nonce);
     const newNonce = oldNonce.slice(0);
diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js
index 45c027d..72541ed 100755
--- a/keepassxc-browser/background/keepass.js
+++ b/keepassxc-browser/background/keepass.js
@@ -193,7 +193,8 @@ keepass.generatePassword = async function(tab) {
         const messageData = {
             action: kpAction,
             nonce: nonce,
-            clientID: keepass.clientID
+            clientID: keepass.clientID,
+            requestID: keepassClient.getRequestId()
         };
 
         const response = await keepassClient.sendMessage(kpAction, tab, messageData, nonce);
diff --git a/keepassxc-protocol.md b/keepassxc-protocol.md
index a724b7a..d6dd4a0 100644
--- a/keepassxc-protocol.md
+++ b/keepassxc-protocol.md
@@ -20,6 +20,7 @@ Encrypted messages are built with these JSON parameters:
 - message - Encrypted message, base64 encoded
 - nonce - 24 bytes long random data, base64 encoded. This is incremented to the response.
 - clientID - 24 bytes long random data, base64 encoded. This is used for a single session to identify different browsers if multiple are used with proxy application.
+- requestID (optional) - A random 8 character string. Used to identify error responses. Currently used only with `generate-password`.
 
 Currently these messages are implemented:
 - `change-public-keys`: Request for passing public keys from client to server and back.
@@ -49,7 +50,7 @@ Response (success):
 ```json
 {
     "action": "change-public-keys",
-    "version": "2.2.0",
+    "version": "2.7.0",
     "publicKey": "<host public key>",
     "success": "true"
 }
@@ -106,7 +107,7 @@ Response message data (success, decrypted):
 ```json
 {
     "hash": "29234e32274a32276e25666a42",
-    "version": "2.2.0",
+    "version": "2.7.0",
     "success": "true",
     "id": "testclient",
     "nonce": "tZvLrBzkQ9GxXq9PvKJj4iAnfPT0VZ3Q"
@@ -136,7 +137,7 @@ Request:
 Response message data (success, decrypted):
 ```json
 {
-    "version": "2.2.0",
+    "version": "2.7.0",
     "nonce": "tZvLrBzkQ9GxXq9PvKJj4iAnfPT0VZ3Q",
     "hash": "29234e32274a32276e25666a42",
     "id": "testclient",
@@ -150,20 +151,16 @@ Request (no unencrypted message is needed):
 {
     "action": "generate-password",
     "nonce": "tZvLrBzkQ9GxXq9PvKJj4iAnfPT0VZ3Q",
-    "clientID": "<clientID>"
+    "clientID": "<clientID>",
+    "requestID": "<request ID>"
 }
 ```
 
 Response message data (success, decrypted):
 ```json
 {
-    "version": "2.2.0",
-    "entries": [
-        {
-            "login": 144,
-            "password": "testclientpassword"
-        }
-    ],
+    "version": "2.7.0",
+    "password": "testclientpassword"
     "success": "true",
     "nonce": "tZvLrBzkQ9GxXq9PvKJj4iAnfPT0VZ3Q"
 }