From cf2d068ed7e4ed4cef6faf54e7ccdb1ab09116e5 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 23 Nov 2017 18:02:07 +0200 Subject: [PATCH] Fixed HTTP auth, decrypt error handling and database-locked response handling --- CHANGELOG | 7 + README.md | 1 + keepassxc-browser/background/httpauth.js | 10 +- keepassxc-browser/background/keepass.js | 233 ++++++++++++--------- keepassxc-browser/background/page.js | 2 +- keepassxc-browser/manifest.json | 2 +- keepassxc-browser/popups/popup_httpauth.js | 20 +- 7 files changed, 159 insertions(+), 116 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 320892e..979cd36 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,10 @@ +0.4.2 (??-??-2017) +========================= +- Fixed HTTP authentication with multiple credentials (credits to smorks) +- Fixed error handling when decrypt fails +- Fixed database-locked response handling +- TODO: Fixed nonce increment when encrypting messages + 0.4.1 (18-11-2017) ========================= - Added support for the credentials dropdown menu with only password field visible diff --git a/README.md b/README.md index 04a9c7b..6b04489 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ The following improvements and features have been made after the fork. At this p - New buttons, icons and settings page graphics - Redesigned password generator dialog - Password generator supports diceware passphrases and extended ASCII characters +- Autocomplete works also when only password fields are visible ## Protocol diff --git a/keepassxc-browser/background/httpauth.js b/keepassxc-browser/background/httpauth.js index 35dc99e..f74277d 100644 --- a/keepassxc-browser/background/httpauth.js +++ b/keepassxc-browser/background/httpauth.js @@ -5,7 +5,7 @@ httpAuth.pendingCallbacks = []; httpAuth.requestCompleted = function(details) { let index = httpAuth.requests.indexOf(details.requestId); - if (index > -1) { + if (index >= 0) { httpAuth.requests.splice(index, 1); } }; @@ -23,6 +23,7 @@ httpAuth.handleRequestCallback = function(details, callback) { httpAuth.processPendingCallbacks = function(details, resolve, reject) { if (httpAuth.requests.indexOf(details.requestId) >= 0 || !page.tabs[details.tabId]) { reject({}); + return; } httpAuth.requests.push(details.requestId); @@ -41,10 +42,7 @@ httpAuth.processPendingCallbacks = function(details, resolve, reject) { httpAuth.loginOrShowCredentials = function(logins, details, resolve, reject) { // at least one login found --> use first to login if (logins.length > 0) { - kpxcEvent.onHTTPAuthPopup(null, { "id": details.tabId }, { "logins": logins, "url": details.searchUrl }); - //generate popup-list for HTTP Auth usernames + descriptions - - if (page.settings.autoFillAndSend) { + if (logins.length == 1 && page.settings.autoFillAndSend) { resolve({ authCredentials: { username: logins[0].login, @@ -52,7 +50,7 @@ httpAuth.loginOrShowCredentials = function(logins, details, resolve, reject) { } }); } else { - reject({}); + kpxcEvent.onHTTPAuthPopup(null, { 'id': details.tabId }, { 'logins': logins, 'url': details.searchUrl, 'resolve': resolve }); } } // no logins found diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index 2a5665d..55b52ca 100644 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -167,11 +167,15 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - callback(keepass.verifyResponse(parsed, response.nonce) ? 'success' : 'error'); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback('error'); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + callback(keepass.verifyResponse(parsed, response.nonce) ? 'success' : 'error'); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -228,25 +232,29 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - - if (keepass.verifyResponse(parsed, response.nonce)) { - entries = parsed.entries; - keepass.updateLastUsed(keepass.databaseHash); - if (entries.length === 0) { - // questionmark-icon is not triggered, so we have to trigger for the normal symbol - browserAction.showDefault(null, tab); - } - callback(entries); - } - else { - console.log('RetrieveCredentials for ' + url + ' rejected'); - } - page.debug('keepass.retrieveCredentials() => entries.length = {1}', entries.length); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback([]); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + entries = parsed.entries; + keepass.updateLastUsed(keepass.databaseHash); + if (entries.length === 0) { + // questionmark-icon is not triggered, so we have to trigger for the normal symbol + browserAction.showDefault(null, tab); + } + callback(entries); + } + else { + console.log('RetrieveCredentials for ' + url + ' rejected'); + } + page.debug('keepass.retrieveCredentials() => entries.length = {1}', entries.length); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -292,25 +300,29 @@ keepass.generatePassword = function(callback, tab, forceCallback) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback([]); + return; + } - if (keepass.verifyResponse(parsed, response.nonce)) { - if (parsed.entries) { - passwords = parsed.entries; - keepass.updateLastUsed(keepass.databaseHash); - } - else { - console.log('No entries returned. Is KeePassXC up-to-date?'); - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + if (parsed.entries) { + passwords = parsed.entries; + keepass.updateLastUsed(keepass.databaseHash); } else { - console.log('GeneratePassword rejected'); + console.log('No entries returned. Is KeePassXC up-to-date?'); } - callback(passwords); } + else { + console.log('GeneratePassword rejected'); + } + callback(passwords); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -352,23 +364,26 @@ keepass.associate = function(callback, tab) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - const id = parsed.id; - - if (!keepass.verifyResponse(parsed, response.nonce)) { - keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); - } - else { - keepass.setCryptoKey(id, key); // Save the current public key as id key for the database - keepass.associated.value = true; - keepass.associated.hash = parsed.hash || 0; - } - - browserAction.show(callback, tab); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + const id = parsed.id; + + if (!keepass.verifyResponse(parsed, response.nonce)) { + keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); + } + else { + keepass.setCryptoKey(id, key); // Save the current public key as id key for the database + keepass.associated.value = true; + keepass.associated.hash = parsed.hash || 0; + } + + browserAction.show(callback, tab); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -434,27 +449,31 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - keepass.isEncryptionKeyUnrecognized = false; + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback(false); + return; + } - if (!keepass.verifyResponse(parsed, response.nonce)) { - const hash = response.hash || 0; - keepass.deleteKey(hash); - keepass.isEncryptionKeyUnrecognized = true; - keepass.handleError(tab, kpErrors.ENCRYPTION_KEY_UNRECOGNIZED); - keepass.associated.value = false; - keepass.associated.hash = null; - } - else if (!keepass.isAssociated()) { - keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); - } - else { - if (tab && page.tabs[tab.id]) { - delete page.tabs[tab.id].errorMessage; - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + keepass.isEncryptionKeyUnrecognized = false; + + if (!keepass.verifyResponse(parsed, response.nonce)) { + const hash = response.hash || 0; + keepass.deleteKey(hash); + keepass.isEncryptionKeyUnrecognized = true; + keepass.handleError(tab, kpErrors.ENCRYPTION_KEY_UNRECOGNIZED); + keepass.associated.value = false; + keepass.associated.hash = null; + } + else if (!keepass.isAssociated()) { + keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); + } + else { + if (tab && page.tabs[tab.id]) { + delete page.tabs[tab.id].errorMessage; } } } @@ -501,30 +520,34 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback('no-hash'); + return; + } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); - if (parsed.hash) { - const oldDatabaseHash = keepass.databaseHash; - keepass.setcurrentKeePassXCVersion(parsed.version); - keepass.databaseHash = parsed.hash || 'no-hash'; + if (parsed.hash) { + const oldDatabaseHash = keepass.databaseHash; + keepass.setcurrentKeePassXCVersion(parsed.version); + keepass.databaseHash = parsed.hash || 'no-hash'; - if (oldDatabaseHash && oldDatabaseHash != keepass.databaseHash) { - keepass.associated.value = false; - keepass.associated.hash = null; - } - - keepass.isDatabaseClosed = false; - keepass.isKeePassXCAvailable = true; - callback(parsed.hash); - } - else if (parsed.errorCode) { - keepass.databaseHash = 'no-hash'; - keepass.isDatabaseClosed = true; - keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); - callback(keepass.databaseHash); + if (oldDatabaseHash && oldDatabaseHash != keepass.databaseHash) { + keepass.associated.value = false; + keepass.associated.hash = null; } + + keepass.isDatabaseClosed = false; + keepass.isKeePassXCAvailable = true; + callback(parsed.hash); + } + else if (parsed.errorCode) { + keepass.databaseHash = 'no-hash'; + keepass.isDatabaseClosed = true; + keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); + callback(keepass.databaseHash); } } else { @@ -604,16 +627,22 @@ keepass.lockDatabase = function(tab) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + resolve(false); + return; + } - if (keepass.verifyResponse(parsed, response.nonce)) { - keepass.isDatabaseClosed = true; - keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); - resolve(false); - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + keepass.isDatabaseClosed = true; + + // Display error message in the popup + keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); + resolve(true); } } else if (response.error && response.errorCode) { diff --git a/keepassxc-browser/background/page.js b/keepassxc-browser/background/page.js index 5b9c525..8a9d5c2 100644 --- a/keepassxc-browser/background/page.js +++ b/keepassxc-browser/background/page.js @@ -48,7 +48,7 @@ page.initOpenedTabs = function() { } // set initial tab-ID - browser.tabs.query({ "active": true, "currentWindow": true }).then((tabs) => { + browser.tabs.query({ 'active': true, 'currentWindow': true }).then((tabs) => { if (tabs.length === 0) { resolve(); return; // For example: only the background devtools or a popup are opened diff --git a/keepassxc-browser/manifest.json b/keepassxc-browser/manifest.json index 98a1b1a..ae67e9e 100644 --- a/keepassxc-browser/manifest.json +++ b/keepassxc-browser/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 2, "name": "keepassxc-browser", - "version": "0.4.1", + "version": "0.4.2", "description": "KeePassXC integration for modern web browsers", "author": "Sami Vänttinen", "icons": { diff --git a/keepassxc-browser/popups/popup_httpauth.js b/keepassxc-browser/popups/popup_httpauth.js index a76a279..7aca550 100644 --- a/keepassxc-browser/popups/popup_httpauth.js +++ b/keepassxc-browser/popups/popup_httpauth.js @@ -1,17 +1,25 @@ $(function() { browser.runtime.getBackgroundPage().then((global) => { - browser.tabs.query({"active": true, "currentWindow": true}).then((tab) => { + browser.tabs.query({'active': true, 'currentWindow': true}).then((tabs) => { + let tab = tabs[0]; const data = global.page.tabs[tab.id].loginList; let ul = document.getElementById('login-list'); for (let i = 0; i < data.logins.length; i++) { const li = document.createElement('li'); const a = document.createElement('a'); - a.textContent = data.logins[i].login + ' (' + data.logins[i].name + ')'; - li.setAttribute('class', 'list-group-item'); + a.textContent = data.logins[i].login + " (" + data.logins[i].name + ")"; li.appendChild(a); - $(a).data('url', data.url.replace(/:\/\//g, '://' + data.logins[i].login + ':' + data.logins[i].password + '@')); - $(a).click(() => { - browser.tabs.update(tab.id, {'url': $(this).data('url')}); + $(a).data('creds', data.logins[i]); + $(a).click(function () { + if (data.resolve) { + const creds = $(this).data('creds'); + data.resolve({ + authCredentials: { + username: creds.login, + password: creds.password + } + }); + } close(); }); ul.appendChild(li);